Views&Matviews with circular dependencies are ignored when distributing

[agedemenli]

1. DeferErrorIfCircularDependencyExists compares each dependency with the object that is passed into the function. See https://github.com/citusdata/citus/blob/main/src/backend/distributed/commands/dependencies.c#L194-L198 An example broken case: We have a dependency chain as: A->B->C<->D. Here we have a circular dependency between C and D, but this function doesn't catch that. The function currently checks if there are circular dependencies between A-B, A-C and A-D, but it doesn't check the pair C-D.

2. GetDependingViews does not add views&matviews with circular dependencies into the result list. See https://github.com/citusdata/citus/blob/main/src/backend/distributed/metadata/dependency.c#L1894-L1895 An example broken case: We have a table t1, two views named v1 and v2 that depend on t1 and each other. We add views&matviews into the result list, only when remainingDependencyCount is zero. But this field never become zero for this case, as they depend on each other.

Luckily, views&matviews on tables to be (distributed/added to metadata) works as expected now, as we simply ignore them when they have circular dependencies. See https://github.com/citusdata/citus/pull/5950/files#diff-eb9f42a2e0e42b7989d6b58b37c564c52a75f6523976b1b719d6c3e8e7020d98R756-R794 Seems like we get the correct result in a wrong way.

For now, as these two items does not break anything, it doesn't seem very urgent. But we should definitely consider writing up the dependency logic around views from scratch.

[agedemenli]

a broken case related to this issue

https://github.com/citusdata/citus/pull/6023#issuecomment-1170951156

[agedemenli]

Now we successfully detect and error out in both cases that are provided in the issue description + the other case linked in my previous comment. Fixed and tested in: https://github.com/citusdata/citus/pull/6051

@onderkalaci I think we can close this issue? Maybe open a new one (if we don't have it already), something like "Support propagating views/matviews with circular dependencies"?

[onderkalaci]

@agedemenli do we already have tests for only-distributed tables involved circular dependencies? If so, yes let's close the issue. Thanks!