On 31.08.21 16:48, Denis Hirn wrote:
The documentation was not up to date anymore with the most recent changes.
This version of the patch fixes that.
I studied up on the theory and terminology being discussed here. I
conclude that what the latest version of this patch is doing (allowing
multiple recursive references, but only in a linear-recursion way) is
sound and useful.
I haven't looked closely at the implementation changes yet. I'm not
very familiar with that part of the code, so it will take a bit longer.
Perhaps you could explain what you are doing there, either in email or
(even better) in the commit message.
What I think this patch needs is a lot more test cases. I would like to
see more variants of different nestings of the UNION branches, some
mixtures of UNION ALL and UNION DISTINCT, joins of the recursive CTE
with regular tables (like in the flights-and-trains examples), as well
as various cases of what is not allowed, namely showing that it can
carefully prohibit non-linear recursion. Also, in one instance you
modify an existing test case. I'm not sure why. Perhaps it would be
better to leave the existing test case alone and write a new one.
You also introduce this concept of reshuffling the UNION branches to
collect all the recursive terms under one subtree. That could use more
testing, like combinations like nonrec UNION rec UNION nonrec UNION rec
etc. Also, currently a query like this works
WITH RECURSIVE t(n) AS (
VALUES (1)
UNION ALL
SELECT n+1 FROM t WHERE n < 100
)
SELECT sum(n) FROM t;
but this doesn't:
WITH RECURSIVE t(n) AS (
SELECT n+1 FROM t WHERE n < 100
UNION ALL
VALUES (1)
)
SELECT sum(n) FROM t;
With your patch, the second should also work, so let's show some tests
for that as well.
