Re: [PATCHES] [HACKERS] WITH RECUSIVE patches 0723

2008-07-25 Thread Tatsuo Ishii
> Thanks for the patch :)
> 
> Now, I get a different problem, this time with the following code
> intended to materialize paths on the fly and summarize down to a
> certain depth in a tree:
> 
> CREATE TABLE tree(
> id INTEGER PRIMARY KEY,
> parent_id INTEGER REFERENCES tree(id)
> );
> 
> INSERT INTO tree
> VALUES (1, NULL), (2, 1), (3,1), (4,2), (5,2), (6,2), (7,3), (8,3),
>(9,4), (10,4), (11,7), (12,7), (13,7), (14, 9), (15,11), (16,11);
> 
> WITH RECURSIVE t(id, path) AS (
> VALUES(1,ARRAY[NULL::integer])
> UNION ALL
> SELECT tree.id, t.path || tree.id
> FROM tree JOIN t ON (tree.parent_id = t.id)
> )
> SELECT
> t1.id, count(t2.*)
> FROM
> t t1
> JOIN
> t t2
> ON (
> t1.path[1:2] = t2.path[1:2]
> AND
> array_upper(t1.path,1) = 2
> AND
> array_upper(t2.path,1) > 2
> )
> GROUP BY t1.id;
> ERROR: unrecognized node type: 203

Thanks for the report. Here is the new patches from Yoshiyuki against
CVS HEAD. Also I have added your test case to the regression test.

> Please apply the attached patch to help out with tab
> completion in psql.

Thanks. Your patches has been included.
--
Tatsuo Ishii
SRA OSS, Inc. Japan


recursive_query.patch.gz
Description: Binary data

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] [HACKERS] WITH RECUSIVE patches 0723

2008-07-24 Thread Tatsuo Ishii
> Now, I get a different problem, this time with the following code
> intended to materialize paths on the fly and summarize down to a
> certain depth in a tree:
> 
> CREATE TABLE tree(
> id INTEGER PRIMARY KEY,
> parent_id INTEGER REFERENCES tree(id)
> );
> 
> INSERT INTO tree
> VALUES (1, NULL), (2, 1), (3,1), (4,2), (5,2), (6,2), (7,3), (8,3),
>(9,4), (10,4), (11,7), (12,7), (13,7), (14, 9), (15,11), (16,11);
> 
> WITH RECURSIVE t(id, path) AS (
> VALUES(1,ARRAY[NULL::integer])
> UNION ALL
> SELECT tree.id, t.path || tree.id
> FROM tree JOIN t ON (tree.parent_id = t.id)
> )
> SELECT
> t1.id, count(t2.*)
> FROM
> t t1
> JOIN
> t t2
> ON (
> t1.path[1:2] = t2.path[1:2]
> AND
> array_upper(t1.path,1) = 2
> AND
> array_upper(t2.path,1) > 2
> )
> GROUP BY t1.id;
> ERROR: unrecognized node type: 203

Thanks for the report. We will look into this.

> Please apply the attached patch to help out with tab
> completion in psql.

Ok, it will appear in the next patches.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] [HACKERS] WITH RECUSIVE patches 0723

2008-07-24 Thread David Fetter
On Thu, Jul 24, 2008 at 01:55:37PM +0900, Tatsuo Ishii wrote:
> > Program received signal SIGSEGV, Segmentation fault.
> 
> Thanks for the report. Here is the new patches from Yoshiyuki.

Thanks for the patch :)

Now, I get a different problem, this time with the following code
intended to materialize paths on the fly and summarize down to a
certain depth in a tree:

CREATE TABLE tree(
id INTEGER PRIMARY KEY,
parent_id INTEGER REFERENCES tree(id)
);

INSERT INTO tree
VALUES (1, NULL), (2, 1), (3,1), (4,2), (5,2), (6,2), (7,3), (8,3),
   (9,4), (10,4), (11,7), (12,7), (13,7), (14, 9), (15,11), (16,11);

WITH RECURSIVE t(id, path) AS (
VALUES(1,ARRAY[NULL::integer])
UNION ALL
SELECT tree.id, t.path || tree.id
FROM tree JOIN t ON (tree.parent_id = t.id)
)
SELECT
t1.id, count(t2.*)
FROM
t t1
JOIN
t t2
ON (
t1.path[1:2] = t2.path[1:2]
AND
array_upper(t1.path,1) = 2
AND
array_upper(t2.path,1) > 2
)
GROUP BY t1.id;
ERROR: unrecognized node type: 203

Please apply the attached patch to help out with tab
completion in psql.

Cheers,
David.
-- 
David Fetter <[EMAIL PROTECTED]> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***
*** 613,621  psql_completion(char *text, int start, int end)
"COMMENT", "COMMIT", "COPY", "CREATE", "DEALLOCATE", "DECLARE",
"DELETE FROM", "DISCARD", "DROP", "END", "EXECUTE", "EXPLAIN", 
"FETCH",
"GRANT", "INSERT", "LISTEN", "LOAD", "LOCK", "MOVE", "NOTIFY", 
"PREPARE",
!   "REASSIGN", "REINDEX", "RELEASE", "RESET", "REVOKE", "ROLLBACK",
"SAVEPOINT", "SELECT", "SET", "SHOW", "START", "TRUNCATE", 
"UNLISTEN",
!   "UPDATE", "VACUUM", "VALUES", NULL
};
  
static const char *const backslash_commands[] = {
--- 613,621 
"COMMENT", "COMMIT", "COPY", "CREATE", "DEALLOCATE", "DECLARE",
"DELETE FROM", "DISCARD", "DROP", "END", "EXECUTE", "EXPLAIN", 
"FETCH",
"GRANT", "INSERT", "LISTEN", "LOAD", "LOCK", "MOVE", "NOTIFY", 
"PREPARE",
!   "REASSIGN", "RECURSIVE", "REINDEX", "RELEASE", "RESET", 
"REVOKE", "ROLLBACK",
"SAVEPOINT", "SELECT", "SET", "SHOW", "START", "TRUNCATE", 
"UNLISTEN",
!   "UPDATE", "VACUUM", "VALUES", "WITH", NULL
};
  
static const char *const backslash_commands[] = {
***
*** 2044,2049  psql_completion(char *text, int start, int end)
--- 2044,2058 
  pg_strcasecmp(prev2_wd, "ANALYZE") == 0))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
  
+ /* WITH [RECURSIVE] */
+   else if (pg_strcasecmp(prev_wd, "WITH") == 0)
+   {
+   static const char *const list_WITH[] =
+   {"RECURSIVE", NULL};
+ 
+   COMPLETE_WITH_LIST(list_WITH);
+   }
+ 
  /* ANALYZE */
/* If the previous word is ANALYZE, produce list of tables */
else if (pg_strcasecmp(prev_wd, "ANALYZE") == 0)

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] [HACKERS] WITH RECUSIVE patches 0723

2008-07-24 Thread Tom Lane
Tatsuo Ishii <[EMAIL PROTECTED]> writes:
> Reviewers, please let me know if you find problems with the
> patches. If none, I would like to commit this weekend.

Given that everyone who has tested this has found a different way to
crash it, and that the frequency of crash reports shows no signs of
slowing down, I have to think that committing it is premature.

I tried to look through the patch just now and failed to make any
sense of it, because of the complete absence of documentation.
Two unexplained examples added to the SELECT reference page don't
do it for me.  I want to see an explanation of exactly what behaviors
are intended to be provided (and, in view of the long TODO list that
was posted awhile back, what isn't provided).  And there needs to be
more than zero internal documentation.  A README file, or perhaps
a very long file header comment, needs to be provided to explain
what's supposed to happen, when, and where when processing a recursive
query.  (For comparison look at the README.HOT file that was created
to explain the HOT patch --- something at about that level of detail
would help this patch a lot.  Or consider adding a section to
chapter 43 in the SGML docs.)

We really can't accept a patch that is so poorly documented as to
be unreviewable.  Unreviewable also means it'll be unmaintainable
going forward.

regards, tom lane

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] [HACKERS] WITH RECUSIVE patches 0723

2008-07-23 Thread Tatsuo Ishii
> On Wed, Jul 23, 2008 at 10:59:20AM -0400, Tom Lane wrote:
> > Tatsuo Ishii <[EMAIL PROTECTED]> writes:
> > > Reviewers, please let me know if you find problems with the
> > > patches. If none, I would like to commit this weekend.
> > 
> > Has this patch actually been reviewed yet?  The only reports I've
> > seen are from testing; nothing from anyone actually reading the
> > code.  I know I've not looked at it yet.
> 
> I've read the code, for what that's worth, which isn't much.  I just
> tried out this patch on a fresh checkout of CVS TIP and found:
> 
> EXPLAIN WITH RECURSIVE t(i) AS (VALUES(1::int) UNION ALL SELECT i+1 FROM t 
> WHERE i < 5) SELECT * FROM t AS t1 JOIN t AS t2 USING(i);
>  QUERY PLAN   
>
> -
>  Hash Join  (cost=0.08..0.16 rows=2 width=4)
>Hash Cond: (t1.i = t2.i)
>->  Recursion on t1  (cost=0.00..0.06 rows=2 width=4)
>  ->  Append  (cost=0.00..0.04 rows=2 width=4)
>->  Values Scan on "*VALUES*"  (cost=0.00..0.01 rows=1 width=4)
>->  Recursive Scan on t  (cost=0.00..0.00 rows=1 width=4)
>  Filter: (i < 5)
>->  Hash  (cost=0.06..0.06 rows=2 width=4)
>  ->  Recursion on t2  (cost=0.00..0.06 rows=2 width=4)
>->  Append  (cost=0.00..0.04 rows=2 width=4)
>  ->  Values Scan on "*VALUES*"  (cost=0.00..0.01 rows=1 
> width=4)
>  ->  Recursive Scan on t  (cost=0.00..0.00 rows=1 width=4)
>Filter: (i < 5)
> (13 rows)
> 
> When I try to execute the query without the EXPLAIN, having attached a 
> debugger
> to the back-end, I get.
> 
> (gdb) continue
> Continuing.
> 
> Program received signal SIGSEGV, Segmentation fault.

Thanks for the report. Here is the new patches from Yoshiyuki. It
appeared that addRangeTableEntryForRecursive() needs to do deep copy
for the subquery and ref in the RangeTblEntry to avoid double free
bug (remember that your example is a self join case). 

Also I added your query to the regression test case with minor
modifications.
--
Tatsuo Ishii
SRA OSS, Inc. Japan


recursive_query.patch.gz
Description: Binary data

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] [HACKERS] WITH RECUSIVE patches 0723

2008-07-23 Thread Tatsuo Ishii
> On Wed, Jul 23, 2008 at 10:59:20AM -0400, Tom Lane wrote:
> > Tatsuo Ishii <[EMAIL PROTECTED]> writes:
> > > Reviewers, please let me know if you find problems with the
> > > patches. If none, I would like to commit this weekend.
> > 
> > Has this patch actually been reviewed yet?  The only reports I've
> > seen are from testing; nothing from anyone actually reading the
> > code.  I know I've not looked at it yet.
> 
> I've read the code, for what that's worth, which isn't much.  I just
> tried out this patch on a fresh checkout of CVS TIP and found:
> 
> EXPLAIN WITH RECURSIVE t(i) AS (VALUES(1::int) UNION ALL SELECT i+1 FROM t 
> WHERE i < 5) SELECT * FROM t AS t1 JOIN t AS t2 USING(i);
>  QUERY PLAN   
>
> -
>  Hash Join  (cost=0.08..0.16 rows=2 width=4)
>Hash Cond: (t1.i = t2.i)
>->  Recursion on t1  (cost=0.00..0.06 rows=2 width=4)
>  ->  Append  (cost=0.00..0.04 rows=2 width=4)
>->  Values Scan on "*VALUES*"  (cost=0.00..0.01 rows=1 width=4)
>->  Recursive Scan on t  (cost=0.00..0.00 rows=1 width=4)
>  Filter: (i < 5)
>->  Hash  (cost=0.06..0.06 rows=2 width=4)
>  ->  Recursion on t2  (cost=0.00..0.06 rows=2 width=4)
>->  Append  (cost=0.00..0.04 rows=2 width=4)
>  ->  Values Scan on "*VALUES*"  (cost=0.00..0.01 rows=1 
> width=4)
>  ->  Recursive Scan on t  (cost=0.00..0.00 rows=1 width=4)
>Filter: (i < 5)
> (13 rows)
> 
> When I try to execute the query without the EXPLAIN, having attached a 
> debugger
> to the back-end, I get.

Thanks for the report. We will look into this.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

> (gdb) continue
> Continuing.
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x08192dcd in ExecQual (qual=0xa183824, econtext=0xa183230, resultForNull=0 
> '\0') at execQual.c:4513
> 4513expr_value = ExecEvalExpr(clause, econtext, &isNull, NULL);
> (gdb) i s
> #0  0x08192dcd in ExecQual (qual=0xa183824, econtext=0xa183230, 
> resultForNull=0 '\0') at execQual.c:4513
> #1  0x08199b23 in ExecScan (node=0xa1831a4, accessMtd=0x81a6bb0 
> ) at execScan.c:131
> #2  0x081a6ba9 in ExecRecursiveScan (node=0xa1831a4) at nodeRecursivescan.c:48
> #3  0x08192320 in ExecProcNode (node=0xa1831a4) at execProcnode.c:380
> #4  0x081a6923 in RecursionNext (node=0xa17fe18) at nodeRecursion.c:68
> #5  0x08199a83 in ExecScan (node=0xa17fe18, accessMtd=0x81a6840 
> ) at execScan.c:68
> #6  0x081a6839 in ExecRecursion (node=0xa17fe18) at nodeRecursion.c:116
> #7  0x081923e0 in ExecProcNode (node=0xa17fe18) at execProcnode.c:339
> #8  0x081a1c13 in MultiExecHash (node=0xa17fcc4) at nodeHash.c:94
> #9  0x081a28b9 in ExecHashJoin (node=0xa17b654) at nodeHashjoin.c:159
> #10 0x081922d8 in ExecProcNode (node=0xa17b654) at execProcnode.c:395
> #11 0x081901db in standard_ExecutorRun (queryDesc=0xa15c334, 
> direction=ForwardScanDirection, count=0) at execMain.c:1271
> #12 0x08240dc4 in PortalRunSelect (portal=0xa15631c, forward=1 '\001', 
> count=0, dest=0xa1733d8) at pquery.c:937
> #13 0x082420e6 in PortalRun (portal=0xa15631c, count=2147483647, isTopLevel=1 
> '\001', dest=0xa1733d8, altdest=0xa1733d8, 
> completionTag=0xbfcacaea "") at pquery.c:793
> #14 0x0823d0a7 in exec_simple_query (
> query_string=0xa12fc9c "WITH RECURSIVE t(i) AS (VALUES(1::int) UNION ALL 
> SELECT i+1 FROM t WHERE i < 5) SELECT * FROM t AS t1 JOIN t AS t2 USING(i);") 
> at postgres.c:977
> #15 0x0823e84c in PostgresMain (argc=4, argv=0xa0d0dac, username=0xa0d0d7c 
> "shackle") at postgres.c:3559
> #16 0x0820957f in ServerLoop () at postmaster.c:3238
> #17 0x0820a4e0 in PostmasterMain (argc=3, argv=0xa0ced50) at postmaster.c:1023
> #18 0x081b69d6 in main (argc=3, argv=0xa0ced50) at main.c:188
> 
> What other information could help track down this problem?
> 
> Cheers,
> David.
> -- 
> David Fetter <[EMAIL PROTECTED]> http://fetter.org/
> Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
> Skype: davidfetter  XMPP: [EMAIL PROTECTED]
> 
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] [HACKERS] WITH RECUSIVE patches 0723

2008-07-23 Thread David Fetter
On Wed, Jul 23, 2008 at 10:59:20AM -0400, Tom Lane wrote:
> Tatsuo Ishii <[EMAIL PROTECTED]> writes:
> > Reviewers, please let me know if you find problems with the
> > patches. If none, I would like to commit this weekend.
> 
> Has this patch actually been reviewed yet?  The only reports I've
> seen are from testing; nothing from anyone actually reading the
> code.  I know I've not looked at it yet.

I've read the code, for what that's worth, which isn't much.  I just
tried out this patch on a fresh checkout of CVS TIP and found:

EXPLAIN WITH RECURSIVE t(i) AS (VALUES(1::int) UNION ALL SELECT i+1 FROM t 
WHERE i < 5) SELECT * FROM t AS t1 JOIN t AS t2 USING(i);
 QUERY PLAN 
 
-
 Hash Join  (cost=0.08..0.16 rows=2 width=4)
   Hash Cond: (t1.i = t2.i)
   ->  Recursion on t1  (cost=0.00..0.06 rows=2 width=4)
 ->  Append  (cost=0.00..0.04 rows=2 width=4)
   ->  Values Scan on "*VALUES*"  (cost=0.00..0.01 rows=1 width=4)
   ->  Recursive Scan on t  (cost=0.00..0.00 rows=1 width=4)
 Filter: (i < 5)
   ->  Hash  (cost=0.06..0.06 rows=2 width=4)
 ->  Recursion on t2  (cost=0.00..0.06 rows=2 width=4)
   ->  Append  (cost=0.00..0.04 rows=2 width=4)
 ->  Values Scan on "*VALUES*"  (cost=0.00..0.01 rows=1 
width=4)
 ->  Recursive Scan on t  (cost=0.00..0.00 rows=1 width=4)
   Filter: (i < 5)
(13 rows)

When I try to execute the query without the EXPLAIN, having attached a debugger
to the back-end, I get.

(gdb) continue
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x08192dcd in ExecQual (qual=0xa183824, econtext=0xa183230, resultForNull=0 
'\0') at execQual.c:4513
4513expr_value = ExecEvalExpr(clause, econtext, &isNull, NULL);
(gdb) i s
#0  0x08192dcd in ExecQual (qual=0xa183824, econtext=0xa183230, resultForNull=0 
'\0') at execQual.c:4513
#1  0x08199b23 in ExecScan (node=0xa1831a4, accessMtd=0x81a6bb0 
) at execScan.c:131
#2  0x081a6ba9 in ExecRecursiveScan (node=0xa1831a4) at nodeRecursivescan.c:48
#3  0x08192320 in ExecProcNode (node=0xa1831a4) at execProcnode.c:380
#4  0x081a6923 in RecursionNext (node=0xa17fe18) at nodeRecursion.c:68
#5  0x08199a83 in ExecScan (node=0xa17fe18, accessMtd=0x81a6840 
) at execScan.c:68
#6  0x081a6839 in ExecRecursion (node=0xa17fe18) at nodeRecursion.c:116
#7  0x081923e0 in ExecProcNode (node=0xa17fe18) at execProcnode.c:339
#8  0x081a1c13 in MultiExecHash (node=0xa17fcc4) at nodeHash.c:94
#9  0x081a28b9 in ExecHashJoin (node=0xa17b654) at nodeHashjoin.c:159
#10 0x081922d8 in ExecProcNode (node=0xa17b654) at execProcnode.c:395
#11 0x081901db in standard_ExecutorRun (queryDesc=0xa15c334, 
direction=ForwardScanDirection, count=0) at execMain.c:1271
#12 0x08240dc4 in PortalRunSelect (portal=0xa15631c, forward=1 '\001', count=0, 
dest=0xa1733d8) at pquery.c:937
#13 0x082420e6 in PortalRun (portal=0xa15631c, count=2147483647, isTopLevel=1 
'\001', dest=0xa1733d8, altdest=0xa1733d8, 
completionTag=0xbfcacaea "") at pquery.c:793
#14 0x0823d0a7 in exec_simple_query (
query_string=0xa12fc9c "WITH RECURSIVE t(i) AS (VALUES(1::int) UNION ALL 
SELECT i+1 FROM t WHERE i < 5) SELECT * FROM t AS t1 JOIN t AS t2 USING(i);") 
at postgres.c:977
#15 0x0823e84c in PostgresMain (argc=4, argv=0xa0d0dac, username=0xa0d0d7c 
"shackle") at postgres.c:3559
#16 0x0820957f in ServerLoop () at postmaster.c:3238
#17 0x0820a4e0 in PostmasterMain (argc=3, argv=0xa0ced50) at postmaster.c:1023
#18 0x081b69d6 in main (argc=3, argv=0xa0ced50) at main.c:188

What other information could help track down this problem?

Cheers,
David.
-- 
David Fetter <[EMAIL PROTECTED]> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] [HACKERS] WITH RECUSIVE patches 0723

2008-07-23 Thread Tatsuo Ishii
> Tatsuo Ishii <[EMAIL PROTECTED]> writes:
> > Reviewers, please let me know if you find problems with the
> > patches. If none, I would like to commit this weekend.
> 
> Has this patch actually been reviewed yet?  The only reports I've
> seen are from testing; nothing from anyone actually reading the
> code.  I know I've not looked at it yet.

The reviewer registered at the Wiki is David Fetter and I believe he
is reading the patches. Michael Makes has contributed the ecpg
part. So apparently he is knowing the ecpg part at least.

I know the patch is huge. Reviewers, please let me know if you have
any question about the code. I would like to do anything for helping
the review.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] [HACKERS] WITH RECUSIVE patches 0723

2008-07-23 Thread Tom Lane
Tatsuo Ishii <[EMAIL PROTECTED]> writes:
> Reviewers, please let me know if you find problems with the
> patches. If none, I would like to commit this weekend.

Has this patch actually been reviewed yet?  The only reports I've
seen are from testing; nothing from anyone actually reading the
code.  I know I've not looked at it yet.

regards, tom lane

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches