From ef65311abba0239920b796386e5a9db547074cfe Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Thu, 14 Mar 2024 17:00:40 +0545 Subject: [PATCH] fix: recursive both ways freezing --- query/config_changes.go | 6 ++-- tests/config_changes_test.go | 8 ++--- tests/config_relationship_test.go | 20 +++++++++++++ views/006_config_views.sql | 49 ++++++++++++++++++------------- 4 files changed, 55 insertions(+), 28 deletions(-) diff --git a/query/config_changes.go b/query/config_changes.go index bf9208e2..633d1c24 100644 --- a/query/config_changes.go +++ b/query/config_changes.go @@ -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"} @@ -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 != "" { diff --git a/tests/config_changes_test.go b/tests/config_changes_test.go index a73faac3..3484316a 100644 --- a/tests/config_changes_test.go +++ b/tests/config_changes_test.go @@ -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)) @@ -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()) @@ -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)) diff --git a/tests/config_relationship_test.go b/tests/config_relationship_test.go index d2b9dd15..371fa56d 100644 --- a/tests/config_relationship_test.go +++ b/tests/config_relationship_test.go @@ -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() { @@ -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() { diff --git a/views/006_config_views.sql b/views/006_config_views.sql index 4b54aa02..569c80e0 100644 --- a/views/006_config_views.sql +++ b/views/006_config_views.sql @@ -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; @@ -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;