Writeable CTEs patch
in [PgSql]
|
From: Alvaro Herrera on 8 Feb 2010 15:26 Robert Haas escribi�: > 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. "DML statements other than SELECT" perhaps? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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: Alvaro Herrera on 8 Feb 2010 15:30 Robert Haas escribi�: > 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. Hmm, how about VALUES? Isn't that a statement on its own right, that would similarly unaffected? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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 8 Feb 2010 13:01 On 2010-02-08 18:42 +0200, Robert Haas wrote: > On Thu, Feb 4, 2010 at 11:57 AM, Marko Tiikkaja >> Here's an updated patch. Only changes from the previous patch are >> fixing the above issue and a regression test for it. > > - 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? No objection to changing that. > - 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. Hmm. Right. That sounds like the right thing to do. Another option (which I seem to recall we've discussed before) is to not allow any SELECT statements between DML WITHs, but I think this is what we should go for. > - 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). Ok, I'll look into that. > - The comment for RegisterSnapshotCopy identifies the function name as > RegisterSnapshot; I think this is a copy-and-pasteo. You're right. Will fix. > - 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. My bison-fu is not exactly strong, but I can look at the feasibility of that. > - 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. I don't see how that would work. We'd still potentially have many different types of DML operations to deal with and that wouldn't help at all at distinguishing which operation actually caused the error. Or did I misunderstand? > 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. 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: Robert Haas on 8 Feb 2010 16:43 On Mon, Feb 8, 2010 at 3:30 PM, Alvaro Herrera <alvherre(a)commandprompt.com> wrote: > Robert Haas escribió: >> 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. > > Hmm, how about VALUES? Isn't that a statement on its own right, that > would similarly unaffected? Ouch. You're right, that's a problem. :-( TABLE is a similar case. ....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: Marko Tiikkaja on 9 Feb 2010 15:13 On 2010-02-08 18:42 +0200, Robert Haas wrote: > On Thu, Feb 4, 2010 at 11:57 AM, 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. > > - 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? I'm going to have to take back my previous statement; this doesn't make a lot of sense in the case of DO ALSO rules (or multiple statements in a DO INSTEAD RULE). Those will have canSetTag=false, but they will be at the top level. > - 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 went ahead and implemented this, but there seems to be one small problem: RECURSIVE. If there is a recursive query between those, it might loop forever even if the top-level SELECT only wanted to see a few rows from it. The docs already discourage writing recursive ctes like that, but still this is a small caveat. > - 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. This seems to work. I used PreparableStmt, but I'm not sure how good idea that really is. Maybe I should create a new one? 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
First
|
Prev
|
Next
|
Last
Pages: 1 2 3 Prev: [HACKERS] pg_class has no toast table? Next: Confusion over Python drivers |