Skip to content

Commit

Permalink
fix: recursive both ways freezing
Browse files Browse the repository at this point in the history
  • Loading branch information
adityathebe authored and moshloop committed Mar 14, 2024
1 parent ac72f6d commit ef65311
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 28 deletions.
6 changes: 3 additions & 3 deletions query/config_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
const (
CatalogChangeRecursiveUpstream = "upstream"
CatalogChangeRecursiveDownstream = "downstream"
CatalogChangeRecursiveBoth = "both"
CatalogChangeRecursiveAll = "all"
)

var allowedConfigChangesSortColumns = []string{"catalog_name", "change_type", "summary", "source", "created_at"}
Expand Down Expand Up @@ -65,8 +65,8 @@ func (t *CatalogChangesSearchRequest) Validate() error {
return fmt.Errorf("catalog id is required")
}

if t.Recursive != "" && !lo.Contains([]string{CatalogChangeRecursiveUpstream, CatalogChangeRecursiveDownstream, CatalogChangeRecursiveBoth}, t.Recursive) {
return fmt.Errorf("recursive must be one of 'upstream', 'downstream' or 'both'")
if t.Recursive != "" && !lo.Contains([]string{CatalogChangeRecursiveUpstream, CatalogChangeRecursiveDownstream, CatalogChangeRecursiveAll}, t.Recursive) {
return fmt.Errorf("recursive must be one of 'upstream', 'downstream' or 'all'")
}

if t.From != "" {
Expand Down
8 changes: 4 additions & 4 deletions tests/config_changes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() {
ginkgo.Context("Both ways", func() {
ginkgo.It("should return changes upstream and downstream", func() {
var relatedChanges []models.ConfigChange
err := DefaultContext.DB().Raw("SELECT * FROM related_changes_recursive(?, 'both')", X.ID).Find(&relatedChanges).Error
err := DefaultContext.DB().Raw("SELECT * FROM related_changes_recursive(?, 'all')", X.ID).Find(&relatedChanges).Error
Expect(err).To(BeNil())

Expect(len(relatedChanges)).To(Equal(4))
Expand Down Expand Up @@ -199,7 +199,7 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() {
ginkgo.It("IN", func() {
response, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{
CatalogID: X.ID,
Recursive: query.CatalogChangeRecursiveBoth,
Recursive: query.CatalogChangeRecursiveAll,
ChangeType: "diff",
})
Expect(err).To(BeNil())
Expand Down Expand Up @@ -290,10 +290,10 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() {
Expect(response.Summary["Pulled"]).To(Equal(1))
})

ginkgo.It(query.CatalogChangeRecursiveBoth, func() {
ginkgo.It(query.CatalogChangeRecursiveAll, func() {
response, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{
CatalogID: V.ID,
Recursive: query.CatalogChangeRecursiveBoth,
Recursive: query.CatalogChangeRecursiveAll,
})
Expect(err).To(BeNil())
Expect(len(response.Changes)).To(Equal(5))
Expand Down
20 changes: 20 additions & 0 deletions tests/config_relationship_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@ var _ = ginkgo.Describe("Config relationship recursive", ginkgo.Ordered, func()
relatedIDs := lo.Map(relatedConfigs, func(rc models.RelatedConfig, _ int) uuid.UUID { return rc.ID })
Expect(relatedIDs).To(ConsistOf([]uuid.UUID{P.ID, M.ID, N.ID, O.ID}))
})

ginkgo.It("recursive both ways", func() {
var relatedConfigs []models.RelatedConfig
err := DefaultContext.DB().Raw("SELECT * FROM related_configs_recursive(?, 'all')", G.ID).Find(&relatedConfigs).Error
Expect(err).To(BeNil())

relatedIDs := lo.Map(relatedConfigs, func(rc models.RelatedConfig, _ int) string { return rc.Name })
Expect(relatedIDs).To(ConsistOf([]string{*D.Name, *B.Name, *H.Name, *A.Name}))
})
})

ginkgo.Context("Cyclic Graph", func() {
Expand Down Expand Up @@ -191,6 +200,17 @@ var _ = ginkgo.Describe("Config relationship recursive", ginkgo.Ordered, func()
Expect(relatedIDs).To(ConsistOf([]uuid.UUID{D.ID, B.ID, A.ID, H.ID}))
})
})

ginkgo.Context("Both", func() {
ginkgo.It("should return parents of a leaf node in a cyclic path", func() {
var relatedConfigs []models.RelatedConfig
err := DefaultContext.DB().Raw("SELECT * FROM related_configs_recursive(?, 'all')", F.ID).Find(&relatedConfigs).Error
Expect(err).To(BeNil())

relatedIDs := lo.Map(relatedConfigs, func(rc models.RelatedConfig, _ int) string { return rc.Name })
Expect(relatedIDs).To(ConsistOf([]string{*A.Name, *C.Name, *H.Name, *D.Name, *B.Name}))
})
})
})

ginkgo.Context("Acyclic Graph", func() {
Expand Down
49 changes: 28 additions & 21 deletions views/006_config_views.sql
Original file line number Diff line number Diff line change
Expand Up @@ -541,25 +541,32 @@ CREATE OR REPLACE FUNCTION related_config_ids_recursive (
include_deleted_configs BOOLEAN DEFAULT FALSE
) RETURNS TABLE (relation_type TEXT, id UUID) AS $$
BEGIN
RETURN query
WITH RECURSIVE all_related_configs AS (
SELECT
rci.relation,
type_filter as relation_type,
rci.id,
ARRAY[config_id] as visited
FROM related_config_ids(config_id, type_filter, include_deleted_configs) rci
IF type_filter = 'incoming' OR type_filter = 'outgoing' THEN
RETURN query
WITH RECURSIVE all_related_configs AS (
SELECT
rci.relation,
type_filter as relation_type,
rci.id,
ARRAY[config_id] as visited
FROM related_config_ids(config_id, type_filter, include_deleted_configs) rci
UNION
SELECT
rc.relation,
type_filter as relation_type,
rc.id,
arc.visited || ARRAY[arc.id, rc.id] as visited
FROM all_related_configs arc
INNER JOIN related_config_ids(arc.id, type_filter, include_deleted_configs) rc
ON rc.id != ALL(arc.visited)
)
SELECT DISTINCT result.relation_type, result.id FROM all_related_configs result;
ELSE
RETURN query
SELECT * FROM related_config_ids_recursive(config_id, 'outgoing', include_deleted_configs)
UNION
SELECT
rc.relation,
type_filter as relation_type,
rc.id,
arc.visited || ARRAY[arc.id, rc.id] as visited
FROM all_related_configs arc
INNER JOIN related_config_ids(arc.id, type_filter, include_deleted_configs) rc
ON rc.id != ALL(arc.visited)
)
SELECT DISTINCT result.relation_type, result.id FROM all_related_configs result;
SELECT * FROM related_config_ids_recursive(config_id, 'incoming', include_deleted_configs);
END IF;
END;
$$ LANGUAGE plpgsql;

Expand Down Expand Up @@ -613,14 +620,14 @@ DROP FUNCTION IF EXISTS related_changes_recursive(UUID, TEXT, BOOLEAN);

CREATE FUNCTION related_changes_recursive (
config_id UUID,
type_filter TEXT DEFAULT 'downstream', -- 'downstream', 'upstream', or 'both'
type_filter TEXT DEFAULT 'downstream', -- 'downstream', 'upstream', or 'all'
include_deleted_configs BOOLEAN DEFAULT FALSE
) RETURNS SETOF config_changes AS $$
BEGIN
RETURN query
SELECT * FROM config_changes cc WHERE
cc.config_id = related_changes_recursive.config_id
OR (($2 = 'downstream' OR $2 = 'both') AND cc.config_id IN (SELECT id FROM related_config_ids_recursive($1, 'outgoing', $3)))
OR (($2 = 'upstream' OR $2 = 'both') AND cc.config_id IN (SELECT id FROM related_config_ids_recursive($1, 'incoming', $3)));
OR (($2 = 'downstream' OR $2 = 'all') AND cc.config_id IN (SELECT id FROM related_config_ids_recursive($1, 'outgoing', $3)))
OR (($2 = 'upstream' OR $2 = 'all') AND cc.config_id IN (SELECT id FROM related_config_ids_recursive($1, 'incoming', $3)));
END;
$$ LANGUAGE plpgsql;

0 comments on commit ef65311

Please sign in to comment.