Monday, July 28, 2008

Re: [HACKERS] WITH RECUSIVE patches 0723

> At David's request I've been looking through this patch.
>
> Regarding documentation: if it would help, I can write some; I have
> already made a start on writing down what is going on internally in
> order to understand it myself.
>
> I've found three more bugs so far:
>
> 1)
>
> create view v2(id) as values (1);
> with recursive t(id) as (select id from v2
> union all select id+1 from t where id < 5)
> select * from t;
> ERROR: could not open relation 1663/16384/24588: No such file or directory
>
> Here it seems that rewriting is simply not being applied to CTEs where
> a recursive clause is present; the reference to "v2" remains in the
> query up until execution time, at which point it errors out (in
> ExecInitSeqScan called from InitPlan).
>
> 2)
>
> with recursive t(id) as (values (1)
> union all select id+1 from t where id < 5
> union all values (2))
> select * from t;
> ERROR: table "t" has 0 columns available but 1 columns specified
>
> This seems to be caused by incorrect assumptions in checkWellFormedCte
> and checkCteSelectStmt (which should have been rejecting the query).
> The query tree as seen by checkWellFormedCte here is (values(1) union
> all select ...) union all (values (2)), and when the left subtree is
> passed to checkCteSelectStmt, it believes it to be non-recursive due
> to the lack of any From clause. The unexpected error is produced
> later.

Included patches from Yoshiyuki should fix 1) and 2). I also add your
SQLs to the regression test. Thanks.

> 3)
>
> with recursive t(id)
> as (values (1)
> union all select t.id+1
> from t left join (values (1)) as s(x) on (false)
> where t.id < 5)
> select * from t;
> id
> ----
> 1
> 2
> (2 rows)
>
> This behaviour is clearly intentional, since the entire mechanism of
> estate->es_disallow_tuplestore exists for no other reason, but it
> seems to me to be clearly wrong. What is the justification for it?

Yes, this is due to prevent infinit recursion caused by following
case for example.

CREATE TABLE test (a TEXT, b TEXT);

INSERT INTO test VALUES ('aaa', 'bbb');
INSERT INTO test VALUES ('bbb', 'ccc');
INSERT INTO test VALUES ('ddd', 'eee');
INSERT INTO test VALUES ('ccc', 'qqq');

WITH RECURSIVE x AS (
SELECT * FROM test WHERE a = 'aaa'

UNION ALL

SELECT test.* FROM x LEFT JOIN test on test.a = x.b
) SELECT * FROM x;

Now we think that we were wrong. This type of query should run into
infinit recursion and it's user's responsibility that he does not make
such a query.

Another idea would be prohibiting *any* outer joins in the recursive
term (DB2 style), but this may be overkill.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

No comments: