This entry has been waiting on author input for a while (our current
threshold is roughly two weeks), so I've marked it Returned with
Feedback.
Once you think the patchset is ready for review again, you (or any
interested party) can resurrect the patch entry by visiting
https://commitfest.pos
Peter Eisentraut writes:
> As I said earlier, I think semantically/mathematically, the changes
> proposed by this patch set are okay.
I took a quick look at this patch because I wondered how it would
affect the SEARCH/CYCLE bug discussed at [1]. Doesn't it break
rewriteSearchAndCycle() complete
The explanations below are satisfactory to me. I think the executor
changes in this patch are ok. But I would like someone else who has
more experience in this particular area to check it too; I'm not going
to take committer responsibility for this by myself without additional
review.
As I
On 14.01.22 15:55, Denis Hirn wrote:
There is nothing in there that says that certain branches of the UNION in a
recursive query mean certain things. In fact, it doesn't even require the query
to contain a UNION at all. It just says to iterate on evaluating the query
until a fixed point is re
> On 14. Jan 2022, at 13:21, Peter Eisentraut
wrote:
>
> There is nothing in there that says that certain branches of the
UNION in a recursive query mean certain things. In fact, it doesn't even
require the query to contain a UNION at all. It just says to iterate on
evaluating the query unti
> On 14. Jan 2022, at 13:21, Peter Eisentraut
> wrote:
>
> There is nothing in there that says that certain branches of the UNION in a
> recursive query mean certain things. In fact, it doesn't even require the
> query to contain a UNION at all. It just says to iterate on evaluating the
>
On 11.01.22 12:33, Denis Hirn wrote:
I have been studying this a bit more. I don't understand your argument here.
Why would this query have different semantics than, say
WITH RECURSIVE t(n) AS (
VALUES (1)
UNION ALL
VALUES (2)
UNION ALL
SELECT n+1 FROM t WHERE n < 1
> I have been studying this a bit more. I don't understand your argument here.
> Why would this query have different semantics than, say
>
> WITH RECURSIVE t(n) AS (
> VALUES (1)
> UNION ALL
> VALUES (2)
> UNION ALL
> SELECT n+1 FROM t WHERE n < 100
> ) SELECT * FROM t LIMI
I have some separate questions on the executor changes. Basically, this
seems the right direction, but I wonder if some things could be clarified.
I wonder why in ExecWorkTableScan() and ExecReScanWorkTableScan() you
call tuplestore_copy_read_pointer() instead of just
tuplestore_select_read_p
On 21.09.21 13:35, Denis Hirn wrote:
Also, currently a query like this works [...] 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
a
> 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.
That's great to hear!
> I haven't looked closely at the implem
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
mult
The documentation was not up to date anymore with the most recent changes.
This version of the patch fixes that.
Best,
–– Denis
0008-Allow-multiple-recursive-self-references.patch
Description: Binary data
> I am not sure if this patch should introduce such a rewriting.
I have thought about this again. I think it is reasonable that this patch
introduces
such a rewriting.
> This well-formedness check apparently needs to be enhanced to allow for more
> than two branches in the union.
The new versi
> This well-formedness check apparently needs to be enhanced to allow for more
> than two branches in the union.
The UNION operators' left associativity causes this problem. Currently, the
recursive term
must be enclosed in parentheses to make this example work:
> WITH RECURSIVE nodes(x) AS (
I tested the following query (from SQLite documentation):
CREATE TABLE edge(aa INT, bb INT);
WITH RECURSIVE nodes(x) AS (
SELECT 59
UNION
SELECT aa FROM edge JOIN nodes ON bb=x
UNION
SELECT bb FROM edge JOIN nodes ON aa=x
)
SELECT x FROM nodes;
ERROR: 42P19: recursive reference
> Maybe the variable selfrefcountL can be renamed slightly (e.g.
> curr_selfrefcount) so that the code is easier to read.
Yes, you are absolutely right. Thanks for the suggestion.
The new version renames this variable.
Best wishes,
-- Denis
0006-Allow-multiple-recursive-self-references.pat
On Tue, Aug 17, 2021 at 5:58 AM Denis Hirn
wrote:
> > The tests fail when you build with assertions enabled (configure
> --enable-cassert).
>
> Thank you for pointing that out. The new version of this patch fixes that.
> The tests are working properly now. All style related issues are fixed as
>
> The tests fail when you build with assertions enabled (configure
> --enable-cassert).
Thank you for pointing that out. The new version of this patch fixes that.
The tests are working properly now. All style related issues are fixed as well.
Best wishes,
-- Denis
0005-Allow-multiple-recurs
On 20.07.21 13:15, Denis Hirn wrote:
In the next version of the patch, would you be so kind as to include
updated documentation of the feature and at least one regression test
of same?
As requested, this new version of the patch contains regression tests and
documentation.
The tests fail whe
> In the next version of the patch, would you be so kind as to include
> updated documentation of the feature and at least one regression test
> of same?
As requested, this new version of the patch contains regression tests and
documentation.
Best wishes,
-- Denis
0004-Allow-multiple-recurs
On Thu, Jul 15, 2021 at 09:18:00AM +0200, Denis Hirn wrote:
> > The patch does not apply on Head anymore, could you rebase and post a patch.
>
> Sure thing. Here's the new patch.
>
> Best wishes,
Thanks for updating this.
In the next version of the patch, would you be so kind as to include
upda
> The patch does not apply on Head anymore, could you rebase and post a patch.
Sure thing. Here's the new patch.
Best wishes,
-- Denis
v3-0001-Allow-multiple-recursive-self-references.patch
Description: Binary data
On Wed, Mar 31, 2021 at 7:28 PM Denis Hirn wrote:
>
> Sorry, I didn't append the patch properly.
The patch does not apply on Head anymore, could you rebase and post a
patch. I'm changing the status to "Waiting for Author".
Regards,
Vignesh
Sorry, I didn't append the patch properly.
Best wishes,
--Denis
v2-0001-Allow-multiple-recursive-self-references.patch
Description: Binary data
Based on Toms feedback, and due to the fact that SQL:2021 forbidsnon-linear recursion, version 2 of the patch allows only linearrecursion. Therefore, later SQL committee decisions on non-linearrecursion should not be problematic.> [LIN] PostgreSQL does not allow multiple references to the recursive
Thanks for the feedback, Tom.
> Tom Lane writes:
> [...]
> As far as I can see, the spec flat-out forbids this. In SQL:2021,
> it's discussed in 7.17 syntax rule 3) j) ix), which
> defines [linear recursion]
(Aside: We don't have a copy of the SQL:2021 specification here (all
we've got here is
Denis Hirn writes:
>> I am not at all sure what the standard says about such recursion [...]
> as far as I know, the standard does not constraint the number of
> self-references
> of recursive common table expressions. However, I could be wrong here.
As far as I can see, the spec flat-out forbi
Hey Pantelis,
> I am not at all sure what the standard says about such recursion [...]
as far as I know, the standard does not constraint the number of self-references
of recursive common table expressions. However, I could be wrong here.
> [...] but it looks like the two t's are treated in your
On Tue, Mar 23, 2021 at 1:03 PM Denis Hirn
wrote:
>
> Hey everyone,
>
> As you know, Postgres currently supports SQL:1999 recursive common table
> expressions, using WITH RECURSIVE. However, Postgres does not allow more
> than
> one recursive self-reference in the recursive term. This restriction
30 matches
Mail list logo
