Thread: CTE bug?
Folks, I tried the following and it broke: WITH RECURSIVE t(j) AS ( WITH RECURSIVE s(i) AS ( VALUES (1) UNION ALL SELECT i+1 FROM s WHERE i < 10 )SELECT i AS j FROM s UNION ALL SELECT j+1 FROM t WHERE j < 10 ) SELECT * FROM t; ERROR: relation "s" does not exist LINE 6: ) SELECT i AS j FROM s ^ Shouldn't this work? Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter <david@fetter.org> writes: > WITH RECURSIVE t(j) AS ( > WITH RECURSIVE s(i) AS ( > VALUES (1) > UNION ALL > SELECT i+1 FROM s WHERE i < 10 > ) SELECT i AS j FROM s > UNION ALL > SELECT j+1 FROM t WHERE j < 10 > ) > SELECT * FROM t; > ERROR: relation "s" does not exist > LINE 6: ) SELECT i AS j FROM s > ^ > Shouldn't this work? Huh, nice test case. It looks like it's trying to do the "throwaway parse analysis" of the nonrecursive term (around line 200 of parse_cte.c) without having analyzed the inner WITH clause. We could probably fix it by doing a throwaway analysis of the inner WITH too ... but ... that whole throwaway thing is pretty ugly and objectionable from a performance standpoint anyhow. I wonder if it wouldn't be better to refactor so that transformSetOperationStmt knows when it's dealing with the body of a recursive UNION and does the analyzeCTETargetList business after having processed the first UNION arm. This would inject a bit more coupling between transformSetOperationStmt and the CTE code than is there now, but it seems to me that if anything it's a less surprising implementation. If you were looking to find where the output column types of a recursive union got determined, you'd expect to find it somewhere near the UNION code, no? regards, tom lane
I wrote: > David Fetter <david@fetter.org> writes: >> WITH RECURSIVE t(j) AS ( >> WITH RECURSIVE s(i) AS ( >> VALUES (1) >> UNION ALL >> SELECT i+1 FROM s WHERE i < 10 >> ) SELECT i AS j FROM s >> UNION ALL >> SELECT j+1 FROM t WHERE j < 10 >> ) >> SELECT * FROM t; >> ERROR: relation "s" does not exist >> LINE 6: ) SELECT i AS j FROM s >> ^ >> Shouldn't this work? > Huh, nice test case. It looks like it's trying to do the "throwaway > parse analysis" of the nonrecursive term (around line 200 of > parse_cte.c) without having analyzed the inner WITH clause. We could > probably fix it by doing a throwaway analysis of the inner WITH too > ... but ... that whole throwaway thing is pretty ugly and objectionable > from a performance standpoint anyhow. I wonder if it wouldn't be better > to refactor so that transformSetOperationStmt knows when it's dealing > with the body of a recursive UNION and does the analyzeCTETargetList > business after having processed the first UNION arm. I've committed a fix along those lines. Too late for 8.4.1 unfortunately :-(. In the meantime, you could work around the problem in this particular case with some more parentheses: WITH RECURSIVE t(j) AS ( ( WITH RECURSIVE s(i) AS ( VALUES (1) UNION ALL SELECT i+1 FROM s WHERE i < 10 ) SELECT i AS j FROM s ) UNION ALL SELECT j+1 FROM t WHERE j < 10 ) SELECT * FROM t; regards, tom lane
On Tue, Sep 08, 2009 at 11:37:14PM -0400, Tom Lane wrote: > I wrote: > > David Fetter <david@fetter.org> writes: > >> WITH RECURSIVE t(j) AS ( > >> WITH RECURSIVE s(i) AS ( > >> VALUES (1) > >> UNION ALL > >> SELECT i+1 FROM s WHERE i < 10 > >> ) SELECT i AS j FROM s > >> UNION ALL > >> SELECT j+1 FROM t WHERE j < 10 > >> ) > >> SELECT * FROM t; > >> ERROR: relation "s" does not exist > >> LINE 6: ) SELECT i AS j FROM s > >> ^ > >> Shouldn't this work? > > > Huh, nice test case. It looks like it's trying to do the > > "throwaway parse analysis" of the nonrecursive term (around line > > 200 of parse_cte.c) without having analyzed the inner WITH clause. > > We could probably fix it by doing a throwaway analysis of the > > inner WITH too ... but ... that whole throwaway thing is pretty > > ugly and objectionable from a performance standpoint anyhow. I > > wonder if it wouldn't be better to refactor so that > > transformSetOperationStmt knows when it's dealing with the body of > > a recursive UNION and does the analyzeCTETargetList business after > > having processed the first UNION arm. > > I've committed a fix along those lines. Too late for 8.4.1 > unfortunately :-(. I just wish I'd found it sooner :) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Tue, Sep 08, 2009 at 11:37:14PM -0400, Tom Lane wrote: > I wrote: > > David Fetter <david@fetter.org> writes: > >> WITH RECURSIVE t(j) AS ( > >> WITH RECURSIVE s(i) AS ( > >> VALUES (1) > >> UNION ALL > >> SELECT i+1 FROM s WHERE i < 10 > >> ) SELECT i AS j FROM s > >> UNION ALL > >> SELECT j+1 FROM t WHERE j < 10 > >> ) > >> SELECT * FROM t; > >> ERROR: relation "s" does not exist > >> LINE 6: ) SELECT i AS j FROM s > >> ^ > >> Shouldn't this work? > > > Huh, nice test case. It looks like it's trying to do the "throwaway > > parse analysis" of the nonrecursive term (around line 200 of > > parse_cte.c) without having analyzed the inner WITH clause. We could > > probably fix it by doing a throwaway analysis of the inner WITH too > > ... but ... that whole throwaway thing is pretty ugly and objectionable > > from a performance standpoint anyhow. I wonder if it wouldn't be better > > to refactor so that transformSetOperationStmt knows when it's dealing > > with the body of a recursive UNION and does the analyzeCTETargetList > > business after having processed the first UNION arm. > > I've committed a fix along those lines. Too late for 8.4.1 > unfortunately :-(. In the meantime, you could work around the > problem in this particular case with some more parentheses: > > WITH RECURSIVE t(j) AS ( > ( > WITH RECURSIVE s(i) AS ( > VALUES (1) > UNION ALL > SELECT i+1 FROM s WHERE i < 10 > ) SELECT i AS j FROM s > ) > UNION ALL > SELECT j+1 FROM t WHERE j < 10 > ) > SELECT * FROM t; > > regards, tom lane I tested this with deeper-nested structures, and ran across another question: Should the outer query be able to reference further-in CTEs? WITH RECURSIVE s(i) AS ( WITH RECURSIVE t(j) AS ( VALUES(1) UNION ALL SELECT j+1 FROM t WHERE j < 10 ) SELECT j AS i FROM t UNION ALL SELECT i+1 FROM s WHERE i < 10 ) SELECT * FROM s,t; ERROR: relation "t" does not exist LINE 11: SELECT * FROM s,t; ^ Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter <david@fetter.org> writes: > Should the outer query be able to reference further-in CTEs? No, why would you expect that? regards, tom lane
On Wed, Sep 09, 2009 at 03:00:39PM -0400, Tom Lane wrote: > David Fetter <david@fetter.org> writes: > > Should the outer query be able to reference further-in CTEs? > > No, why would you expect that? No particular reason, I suppose. I'm not clear on what the standard says about this. Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter <david@fetter.org> writes: > On Wed, Sep 09, 2009 at 03:00:39PM -0400, Tom Lane wrote: >> David Fetter <david@fetter.org> writes: >>> Should the outer query be able to reference further-in CTEs? >> >> No, why would you expect that? > No particular reason, I suppose. I'm not clear on what the standard > says about this. The standard says that the scope of a WITH is the <query expression body> it precedes. regards, tom lane