Writeable CTEs patch
in [PgSql]
|
From: Takahiro Itagaki on 5 Feb 2010 00:14 Marko Tiikkaja <marko.tiikkaja(a)cs.helsinki.fi> wrote: > Here's an updated patch. Only changes from the previous patch are > fixing the above issue and a regression test for it. A brief report for of the patch: * The patch has the following error cases, and also have one regression test for each case. - DML WITH is not allowed in a cursor declaration - DML WITH is not allowed in a view definition - DML WITH without RETURNING is only allowed inside an unreferenced CTE - DML WITH is only allowed at the top level - Recursive DML WITH statements are not supported ^-- might be better if "DML WITH cannot have the self-reference" or so? - Conditional DO INSTEAD rules are not supported in DML WITH statements - DO ALSO rules are not supported in DML WITH statements - Multi-statement DO INSTEAD rules are not supported in DML WITH statements - DO INSTEAD NOTHING rules are not supported in DML WITH statements * In the regression tests, almost all of them don't have ORDER BY clause. They just work, but we might need ORDER BY to get robust output. What did we do in other regression tests? * I feel odd the following paragraph in the docs, but should be checked by native English speakers. *** a/doc/src/sgml/ref/create_rule.sgml --- b/doc/src/sgml/ref/create_rule.sgml *************** *** 222,227 **** CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS --- 222,234 ---- </para> <para> + In an <literal>INSERT</literal>, <literal>UPDATE</literal> or + <literal>DELETE</literal> query within a <literal>WITH</literal> clause, + only unconditional, single-statement <literal>INSTEAD</literal> rules are ^-- and? which comma is the sentence separator? + implemented. ^-- might be "available" rather than "implemented"? + </para> Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Marko Tiikkaja on 5 Feb 2010 00:47 On 2010-02-05 07:14 UTC+2, Takahiro Itagaki wrote: > > Marko Tiikkaja <marko.tiikkaja(a)cs.helsinki.fi> wrote: > >> Here's an updated patch. Only changes from the previous patch are >> fixing the above issue and a regression test for it. > > * In the regression tests, almost all of them don't have ORDER BY clause. > They just work, but we might need ORDER BY to get robust output. > What did we do in other regression tests? Looking at with.sql, it seems to use ORDER BY when it accesses data from a table. But obviously we can't do this if want to test INSERT/UPDATE/DELETE .. RETURNING at the top level and returning.sql seems to be relying on the fact that they come out in the same order every time. Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 5 Feb 2010 01:03 Takahiro Itagaki <itagaki.takahiro(a)oss.ntt.co.jp> writes: > * In the regression tests, almost all of them don't have ORDER BY clause. > They just work, but we might need ORDER BY to get robust output. > What did we do in other regression tests? We add ORDER BY only when experience shows it's necessary. The reasoning is explained in regress.sgml: You might wonder why we don't order all the regression test queries explicitly to get rid of this issue once and for all. The reason is that that would make the regression tests less useful, not more, since they'd tend to exercise query plan types that produce ordered results to the exclusion of those that don't. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 8 Feb 2010 11:42 On Thu, Feb 4, 2010 at 11:57 AM, Marko Tiikkaja <marko.tiikkaja(a)cs.helsinki.fi> wrote: > On 2010-02-04 18:04 UTC+2, I wrote: >> While working on the docs, I noticed one problem with the patch itself: >> it doesn't handle multi-statement DO INSTEAD rules correctly. I'm going >> to submit a fix for that later. > > Here's an updated patch. Only changes from the previous patch are > fixing the above issue and a regression test for it. The comments on the parts I asked about before are much better in this version. A few other things that I notice: - I'm not sure that canSetTag is the right name for the additional argument to ExecInsert/ExecUpdate/ExecDelete. OTOH, I'm not sure it's the wrong name either. But should we use something like isTopLevelQuery? - It appears that we pull out all of the DML statements first and run them in order, but I'm not sure that's the right thing to do. Consider: WITH x AS (INSERT ...), y AS (SELECT ...), z AS (INSERT ...) SELECT ... I would assume we would do x, CCI, do y, do z, CCI, do main query, but I don't think that's what this implements. The user might be surprised to find out that y sees the effects of z. - I think that the comment in analyzeCTE that says /* Check that we got something reasonable */ could be fleshed out a bit. You could still reference transformRangeSubselect, for example, but then explain why the checks here are different (viz, CTEs can contain DML). - The comment for RegisterSnapshotCopy identifies the function name as RegisterSnapshot; I think this is a copy-and-pasteo. - It seems like the gram.y changes for common_table_expr might benefit from some factoring; that is, create a production (or find a suitable existing one) for "statements of the sort that can appear within CTEs", and then use that in common_table_expr. Or maybe this doesn't work; I haven't tried it. - I still don't much like the idea of using DML WITH in error messages. One idea I had (which might suck, but I'm just throwing it out there) is to change hasDmlWith to an integer bitmap with a bit for each of insert, update, and delete. But it may be better still to just rephrase the error messages. Could we just write, e.g. "non-SELECT statements are not allowed within a cursor declaration?" Or we could say "INSERT, UPDATE, and DELETE statements are not allowed within a cursor declaration", but I'm thinking we may want to allow things like COPY and EXPLAIN inside CTEs in the future, too, and they'll presumably be treated similarly to DML. For the record, Tom or whoever should feel to swoop in here at any time, or add to any of this. I'm just making suggestions until the big guns show up. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 8 Feb 2010 15:18 On Mon, Feb 8, 2010 at 1:01 PM, Marko Tiikkaja <marko.tiikkaja(a)cs.helsinki.fi> wrote: >> Could we just write, e.g. >> "non-SELECT statements are not allowed within a cursor declaration?" >> Or we could say "INSERT, UPDATE, and DELETE statements are not allowed >> within a cursor declaration", but I'm thinking we may want to allow >> things like COPY and EXPLAIN inside CTEs in the future, too, and >> they'll presumably be treated similarly to DML. > > "INSERT, UPDATE and DELETE" is quite long and "non-SELECT" is a bit > clumsy IMO. But I don't really have anything better to offer, either. Yeah, I don't feel good about "INSERT, UPDATE, and DELETE" because in most of the relevant contexts the list might get longer if in the future we allow things like EXPLAIN and COPY within CTEs. I think "Non-SELECT statement" is reasonably clear, though; people might not know which things are statements, but the message implies that SELECT is one such thing, and not the one that's the problem, which should get them pointed in the right direction. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
|
Next
|
Last
Pages: 1 2 3 Prev: [HACKERS] pg_class has no toast table? Next: Confusion over Python drivers |