Re: [PATCHES] DELETE ... USING
Neil Conway <[EMAIL PROTECTED]> writes: > Well, my previous message described why I'm not sure that this line of > reasoning is correct. I think the only really proper configuration is > add_missing_from=false and an explicit USING/FROM list. Just about the > only reason to enable add_missing_from would be for compatibility with > previous releases of PostgreSQL -- and that "compatible" behavior is not > to issue a warning for UPDATE and DELETE in this situation. Hmm. There's some merit in that position, but consider this: we are encouraging people rather strongly to move to the add_missing_from=false behavior. So add_missing_from=true could be seen as a testing situation in which you'd like to know which of your queries have a problem, while not actually causing your app to fail. Strict backwards compatibility won't produce the warning but also won't help you find what will break. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] DELETE ... USING
Tom Lane wrote: ... but when it is TRUE, there should be a notice, same as there is in SELECT. UPDATE should produce such a notice too, IMHO. Probably we omitted the message originally because there was no way to avoid it in a DELETE, but now there will be. Well, my previous message described why I'm not sure that this line of reasoning is correct. I think the only really proper configuration is add_missing_from=false and an explicit USING/FROM list. Just about the only reason to enable add_missing_from would be for compatibility with previous releases of PostgreSQL -- and that "compatible" behavior is not to issue a warning for UPDATE and DELETE in this situation. If the user deliberately enables add_missing_from, I'm inclined to trust them that they know what they're doing. -Neil ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] DELETE ... USING
Neil Conway <[EMAIL PROTECTED]> writes: > Euler Taveira de Oliveira wrote: >> euler=# delete from t1 where t1.a = t3.x; >> DELETE 1 >> euler=# >> >> I think we need at least a NOTICE here. Of course it could be extended >> to UPDATE too. > I can see an argument for having a NOTICE here. On the other hand, > add_missing_from will default to false in 8.1, ... ... but when it is TRUE, there should be a notice, same as there is in SELECT. UPDATE should produce such a notice too, IMHO. Probably we omitted the message originally because there was no way to avoid it in a DELETE, but now there will be. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] DELETE ... USING
Euler Taveira de Oliveira wrote: I'm worried about add_missing_from enabled. The plan is to make add_missing_from default to false in 8.1 euler=# delete from t3 using t1 where b > 500; DELETE 4 euler=# select * from t3; x | y ---+--- (0 rows) In this case, I 'forget' to do the join and it delete all rows from t3. I know that user needs to pay attention, but ... What about default add_missing_from to off? add_missing_from would not make any difference here. The problem is that there is no join clause between t3 and t1, not that t1 is being implicitly added to the range table (which is what add_missing_from would warn you about). The problem is analogous to a SELECT like: SELECT * FROM t3, t1 WHERE b > 500; i.e. forgetting to specify a join clause and therefore accidentally computing the cartesian product. There has been some gripping recently on -hackers about disabling this or emitting a warning of some kind. euler=# select * from t1 where t1.a = t3.x; NOTICE: adding missing FROM-clause entry for table "t3" NOTICE: adding missing FROM-clause entry for table "t3" a | b ---+ 5 | 10 (1 row) euler=# delete from t1 where t1.a = t3.x; DELETE 1 euler=# I think we need at least a NOTICE here. Of course it could be extended to UPDATE too. I can see an argument for having a NOTICE here. On the other hand, add_missing_from will default to false in 8.1, so presumably the only people enabling it will be those who specifically need backward compatibility for old applications that they cannot afford to change. Filling the logs with bogus NOTICEs would be sufficiently annoying it would probably force some people to modify their applications, thereby defeating the point of having a backward compatibility GUC variable in the first place. BTW, what about regression tests for UPDATE ... FROM? I agree regression tests would be useful -- you are welcome to send a patch :) -Neil ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] DELETE ... USING
Hi Neil, > > BTW, this patch is lacking ruleutils.c support. Put a DELETE USING > > into a rule and see whether pg_dump will dump the rule correctly > ... > > Good catch; a revised patch is attached. > Greate job. But I'm worried about add_missing_from enabled. See: euler=# delete from t3 using t1 where b > 500; DELETE 4 euler=# select * from t3; x | y ---+--- (0 rows) In this case, I 'forget' to do the join and it delete all rows from t3. I know that user needs to pay attention, but ... What about default add_missing_from to off? The other case is: euler=# select * from t1 where t1.a = t3.x; NOTICE: adding missing FROM-clause entry for table "t3" NOTICE: adding missing FROM-clause entry for table "t3" a | b ---+ 5 | 10 (1 row) euler=# delete from t1 where t1.a = t3.x; DELETE 1 euler=# I think we need at least a NOTICE here. Of course it could be extended to UPDATE too. BTW, what about regression tests for UPDATE ... FROM? PS> all examples are extracted from regression database. Euler Taveira de Oliveira euler[at]yahoo_com_br Yahoo! Acesso Grátis - Internet rápida e grátis. Instale o discador agora! http://br.acesso.yahoo.com/ ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] avg(int2) and avg(int8) micro-opt
Neil Conway <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Why only those two? Might as well make all the accum functions look alike. > Yeah, there might be some others we could improve. float4_accum() and > float8_accum() look like they could be improved pretty easily, and > do_numeric_accum() should also be fixable with some hackery. I suppose > it's also worth optimizing int2_sum(), int4_sum() and int8_sum(). I'll > send a patch for this later today or tomorrow. Are there any other > transition functions where we can apply this technique? Actually, do_numeric_accum can't be fixed easily because numeric is a varlena type. The basic requirement for this hack is that the size of the transition value be constant ... regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] DELETE ... USING
[ CC'ing hackers to see if anyone else wants to weigh in ] Tom Lane wrote: Of course, the entire reason this didn't happen years ago is that we couldn't agree on what keyword to use... you sure you want to reopen that discussion? Sure, it doesn't seem too difficult to settle to me. I don't think changing UPDATE is a good idea. It's consistent with SELECT and people are used to it. Fair enough, I can't get too excited about it either. You could argue that something like DELETE FROM target [ { USING | FROM } othertables ] ... is the best compromise. Those who like consistency can write FROM, those who don't like "FROM a FROM b" can write something else. This would be fine with me. Are there any other opinions out there on what syntax would be best for this feature? (For those on -hackers, the feature in question is adding the ability to specify additional tables to "join" against in a DELETE, as can be done using FROM in UPDATE.) -Neil ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] DELETE ... USING
Tom Lane wrote: BTW, this patch is lacking ruleutils.c support. Put a DELETE USING into a rule and see whether pg_dump will dump the rule correctly ... Good catch; a revised patch is attached. -Neil Index: doc/src/sgml/ref/delete.sgml === RCS file: /var/lib/cvs/pgsql/doc/src/sgml/ref/delete.sgml,v retrieving revision 1.22 diff -c -r1.22 delete.sgml *** doc/src/sgml/ref/delete.sgml 9 Jan 2005 05:57:45 - 1.22 --- doc/src/sgml/ref/delete.sgml 5 Apr 2005 00:42:17 - *** *** 20,26 ! DELETE FROM [ ONLY ] table [ WHERE condition ] --- 20,28 ! DELETE FROM [ ONLY ] table ! [ USING usinglist ] ! [ WHERE condition ] *** *** 50,58 You must have the DELETE privilege on the table to delete from it, as well as the SELECT !privilege for any table whose values are read in the condition. --- 52,69 +There are two ways to delete rows in a table using information +contained in other tables in the database: using sub-selects, or +specifying additional tables in the USING clause. +Which technique is more appropriate depends on the specific +circumstances. + + + You must have the DELETE privilege on the table to delete from it, as well as the SELECT !privilege for any table in the USING clause or !whose values are read in the condition. *** *** 71,76 --- 82,101 + usinglist + + + A list of table expressions, allowing columns from other tables + to appear in the WHERE condition. This is similar + to the list of tables that can be specified in the of a + SELECT statement; for example, an alias for + the table name can be specified. + + + + + condition *** *** 105,114 PostgreSQL lets you reference columns of !other tables in the WHERE condition. For example, to !delete all films produced by a given producer, one might do ! DELETE FROM films WHERE producer_id = producers.id AND producers.name = 'foo'; What is essentially happening here is a join between films --- 130,140 PostgreSQL lets you reference columns of !other tables in the WHERE condition by specifying the !other tables in the USING clause. For example, !to delete all films produced by a given producer, one might do ! DELETE FROM films USING producers WHERE producer_id = producers.id AND producers.name = 'foo'; What is essentially happening here is a join between films *** *** 120,129 WHERE producer_id IN (SELECT id FROM producers WHERE name = 'foo'); In some cases the join style is easier to write or faster to !execute than the sub-select style. One objection to the join style !is that there is no explicit list of what tables are being used, !which makes the style somewhat error-prone; also it cannot handle !self-joins. --- 146,158 WHERE producer_id IN (SELECT id FROM producers WHERE name = 'foo'); In some cases the join style is easier to write or faster to !execute than the sub-select style. ! ! ! !If add_missing_from is enabled, any relations !mentioned in the WHERE condition will be !implicitly added to the USING clause. *** *** 149,157 Compatibility !This command conforms to the SQL standard, except that the ability to !reference other tables in the WHERE clause is a !PostgreSQL extension. --- 178,187 Compatibility !This command conforms to the SQL standard, except that the !USING clause and the ability to reference other tables !in the WHERE clause are PostgreSQL !extensions. Index: src/backend/nodes/copyfuncs.c === RCS file: /var/lib/cvs/pgsql/src/backend/nodes/copyfuncs.c,v retrieving revision 1.299 diff -c -r1.299 copyfuncs.c *** src/backend/nodes/copyfuncs.c 29 Mar 2005 17:58:50 - 1.299 --- src/backend/nodes/copyfuncs.c 5 Apr 2005 00:42:17 - *** *** 1578,1583 --- 1578,1584 COPY_NODE_FIELD(relation); COPY_NODE_FIELD(whereClause); + COPY_NODE_FIELD(usingClause); return newnode; } Index: src/backend/nodes/equalfuncs.c === RCS file: /var/lib/cvs/pgsql/src/backend/nodes/equalfuncs.c,v retrieving revision 1.238 diff -c -r1.238 equalfuncs.c *** src/backend/nodes/equalfuncs.c 29 Mar 2005 17:58:50 - 1.238 --- src/backend/nodes/equalfuncs.c 5 Apr 2005 00:42:17 - *** *** 685,690 --- 685,691 { COMPARE_NODE_FIELD(relation); COMPARE_NODE_FIELD(whereClause); + COMPARE_NODE_FIELD(usi
Re: [PATCHES] avg(int2) and avg(int8) micro-opt
Tom Lane wrote: Why only those two? Might as well make all the accum functions look alike. Yeah, there might be some others we could improve. float4_accum() and float8_accum() look like they could be improved pretty easily, and do_numeric_accum() should also be fixable with some hackery. I suppose it's also worth optimizing int2_sum(), int4_sum() and int8_sum(). I'll send a patch for this later today or tomorrow. Are there any other transition functions where we can apply this technique? BTW, int2_avg_accum() and int4_avg_accum() patch applied to HEAD. -Neil ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] DELETE ... USING
BTW, this patch is lacking ruleutils.c support. Put a DELETE USING into a rule and see whether pg_dump will dump the rule correctly ... regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] DELETE ... USING
Neil Conway <[EMAIL PROTECTED]> writes: > On a related note, UPDATE uses the FROM keyword to denote the list of > relations to join with, whereas DELETE uses USING. Should we make USING > an alias for FROM in UPDATE and if so, should we deprecate FROM? This > would be more consistent, which I suppose is a good thing. Of course, the entire reason this didn't happen years ago is that we couldn't agree on what keyword to use... you sure you want to reopen that discussion? I don't think changing UPDATE is a good idea. It's consistent with SELECT and people are used to it. You could argue that something like DELETE FROM target [ { USING | FROM } othertables ] ... is the best compromise. Those who like consistency can write FROM, those who don't like "FROM a FROM b" can write something else. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] avg(int2) and avg(int8) micro-opt
Neil Conway <[EMAIL PROTECTED]> writes: > This patch changes int2_avg_accum() and int4_avg_accum() use the nodeAgg > performance hack Tom introduced recently. Why only those two? Might as well make all the accum functions look alike. > It is possible that the transition array might be TOAST'ed (not that I'd > expect that to occur in practice, of course). AFAICS that is impossible, since the transition value is never stored to disk. But your analysis is good anyway. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
[PATCHES] avg(int2) and avg(int8) micro-opt
This patch changes int2_avg_accum() and int4_avg_accum() use the nodeAgg performance hack Tom introduced recently. This means we can avoid copying the transition array for each input tuple if these functions are invoked as aggregate transition functions. To test the performance improvement, I created a 1 million row table with a single int4 column. Without the patch, SELECT avg(col) FROM table took about 4.2 seconds (after the data was cached); with the patch, it took about 3.2 seconds. Naturally, the performance improvement for a less trivial query (or a table with wider rows) would be relatively smaller. It is possible that the transition array might be TOAST'ed (not that I'd expect that to occur in practice, of course). The aggregates should continue to work in this case: PG_DETOAST_DATUM() is equivalent to PG_DETOAST_DATUM_COPY() if the datum is toast'ed, so in effect we just won't implement the nodeAgg performance hack if the transition array is toasted. If I've mucked this up, let me know. Barring any objections, I'll commit this tomorrow. -Neil Index: src/backend/utils/adt/numeric.c === RCS file: /Users/neilc/local/cvs/pgsql/src/backend/utils/adt/numeric.c,v retrieving revision 1.81 diff -c -r1.81 numeric.c *** src/backend/utils/adt/numeric.c 1 Jan 2005 05:43:07 - 1.81 --- src/backend/utils/adt/numeric.c 4 Apr 2005 11:00:41 - *** *** 2462,2478 Datum int2_avg_accum(PG_FUNCTION_ARGS) { ! ArrayType *transarray = PG_GETARG_ARRAYTYPE_P_COPY(0); int16 newval = PG_GETARG_INT16(1); Int8TransTypeData *transdata; /* ! * We copied the input array, so it's okay to scribble on it directly. */ if (ARR_SIZE(transarray) != ARR_OVERHEAD(1) + sizeof(Int8TransTypeData)) elog(ERROR, "expected 2-element int8 array"); - transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray); transdata->count++; transdata->sum += newval; --- 2462,2485 Datum int2_avg_accum(PG_FUNCTION_ARGS) { ! ArrayType *transarray; int16 newval = PG_GETARG_INT16(1); Int8TransTypeData *transdata; /* ! * If we're invoked by nodeAgg, we can cheat and modify our first ! * parameter in-place to reduce palloc overhead. Otherwise we need ! * to make a copy of it before scribbling on it. */ + if (fcinfo->context && IsA(fcinfo->context, AggState)) + transarray = PG_GETARG_ARRAYTYPE_P(0); + else + transarray = PG_GETARG_ARRAYTYPE_P_COPY(0); + if (ARR_SIZE(transarray) != ARR_OVERHEAD(1) + sizeof(Int8TransTypeData)) elog(ERROR, "expected 2-element int8 array"); + transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray); transdata->count++; transdata->sum += newval; *** *** 2482,2498 Datum int4_avg_accum(PG_FUNCTION_ARGS) { ! ArrayType *transarray = PG_GETARG_ARRAYTYPE_P_COPY(0); int32 newval = PG_GETARG_INT32(1); Int8TransTypeData *transdata; /* ! * We copied the input array, so it's okay to scribble on it directly. */ if (ARR_SIZE(transarray) != ARR_OVERHEAD(1) + sizeof(Int8TransTypeData)) elog(ERROR, "expected 2-element int8 array"); - transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray); transdata->count++; transdata->sum += newval; --- 2489,2512 Datum int4_avg_accum(PG_FUNCTION_ARGS) { ! ArrayType *transarray; int32 newval = PG_GETARG_INT32(1); Int8TransTypeData *transdata; /* ! * If we're invoked by nodeAgg, we can cheat and modify our first ! * parameter in-place to reduce palloc overhead. Otherwise we need ! * to make a copy of it before scribbling on it. */ + if (fcinfo->context && IsA(fcinfo->context, AggState)) + transarray = PG_GETARG_ARRAYTYPE_P(0); + else + transarray = PG_GETARG_ARRAYTYPE_P_COPY(0); + if (ARR_SIZE(transarray) != ARR_OVERHEAD(1) + sizeof(Int8TransTypeData)) elog(ERROR, "expected 2-element int8 array"); + transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray); transdata->count++; transdata->sum += newval; ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] DELETE ... USING
This patch is a cleaned up version of Euler Taveira de Oliveira's patch implementing DELETE ... USING. I removed a bunch of unused code (no need to tlist transformations), updated copyfuncs/equalfuncs, improved the documentation, rearranged a few things, and added regression tests. I haven't done psql tab completion. Barring any objections, I'll apply this to HEAD tomorrow. On a related note, UPDATE uses the FROM keyword to denote the list of relations to join with, whereas DELETE uses USING. Should we make USING an alias for FROM in UPDATE and if so, should we deprecate FROM? This would be more consistent, which I suppose is a good thing. -Neil Index: doc/src/sgml/ref/delete.sgml === RCS file: /Users/neilc/local/cvs/pgsql/doc/src/sgml/ref/delete.sgml,v retrieving revision 1.22 diff -c -r1.22 delete.sgml *** doc/src/sgml/ref/delete.sgml 9 Jan 2005 05:57:45 - 1.22 --- doc/src/sgml/ref/delete.sgml 4 Apr 2005 10:10:42 - *** *** 20,26 ! DELETE FROM [ ONLY ] table [ WHERE condition ] --- 20,28 ! DELETE FROM [ ONLY ] table ! [ USING usinglist ] ! [ WHERE condition ] *** *** 50,58 You must have the DELETE privilege on the table to delete from it, as well as the SELECT !privilege for any table whose values are read in the condition. --- 52,69 +There are two ways to delete rows in a table using information +contained in other tables in the database: using sub-selects, or +specifying additional tables in the USING clause. +Which technique is more appropriate depends on the specific +circumstances. + + + You must have the DELETE privilege on the table to delete from it, as well as the SELECT !privilege for any table in the USING clause or !whose values are read in the condition. *** *** 71,76 --- 82,101 + usinglist + + + A list of table expressions, allowing columns from other tables + to appear in the WHERE condition. This is similar + to the list of tables that can be specified in the of a + SELECT statement; for example, an alias for + the table name can be specified. + + + + + condition *** *** 105,114 PostgreSQL lets you reference columns of !other tables in the WHERE condition. For example, to !delete all films produced by a given producer, one might do ! DELETE FROM films WHERE producer_id = producers.id AND producers.name = 'foo'; What is essentially happening here is a join between films --- 130,140 PostgreSQL lets you reference columns of !other tables in the WHERE condition by specifying the !other tables in the USING clause. For example, !to delete all films produced by a given producer, one might do ! DELETE FROM films USING producers WHERE producer_id = producers.id AND producers.name = 'foo'; What is essentially happening here is a join between films *** *** 120,129 WHERE producer_id IN (SELECT id FROM producers WHERE name = 'foo'); In some cases the join style is easier to write or faster to !execute than the sub-select style. One objection to the join style !is that there is no explicit list of what tables are being used, !which makes the style somewhat error-prone; also it cannot handle !self-joins. --- 146,158 WHERE producer_id IN (SELECT id FROM producers WHERE name = 'foo'); In some cases the join style is easier to write or faster to !execute than the sub-select style. ! ! ! !If add_missing_from is enabled, any relations !mentioned in the WHERE condition will be !implicitly added to the USING clause. *** *** 149,157 Compatibility !This command conforms to the SQL standard, except that the ability to !reference other tables in the WHERE clause is a !PostgreSQL extension. --- 178,187 Compatibility !This command conforms to the SQL standard, except that the !USING clause and the ability to reference other tables !in the WHERE clause are PostgreSQL !extensions. Index: src/backend/nodes/copyfuncs.c === RCS file: /Users/neilc/local/cvs/pgsql/src/backend/nodes/copyfuncs.c,v retrieving revision 1.299 diff -c -r1.299 copyfuncs.c *** src/backend/nodes/copyfuncs.c 29 Mar 2005 17:58:50 - 1.299 --- src/backend/nodes/copyfuncs.c 4 Apr 2005 07:56:36 - *** *** 1578,1583 --- 1578,1584 COPY_NODE_FIELD(relation); COPY_NODE_FIELD(whereClause); + COPY_NODE_FIELD(usingClause); return newnode; } Index: src/backe
[PATCHES] trivial tab complete fixes
I've applied the attached patch to HEAD: it fixes the spelling of the "ABSOLUTE" keyword, and completes FETCH with FROM and IN, rather than FROM and TO (since the latter is not valid syntax). -Neil Index: src/bin/psql/tab-complete.c === RCS file: /Users/neilc/local/cvs/pgsql/src/bin/psql/tab-complete.c,v retrieving revision 1.122 diff -c -r1.122 tab-complete.c *** src/bin/psql/tab-complete.c 19 Mar 2005 23:27:08 - 1.122 --- src/bin/psql/tab-complete.c 4 Apr 2005 07:10:03 - *** *** 1186,1192 pg_strcasecmp(prev_wd, "MOVE") == 0) { static const char *const list_FETCH1[] = ! {"ABSOLUT", "BACKWARD", "FORWARD", "RELATIVE", NULL}; COMPLETE_WITH_LIST(list_FETCH1); } --- 1186,1192 pg_strcasecmp(prev_wd, "MOVE") == 0) { static const char *const list_FETCH1[] = ! {"ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE", NULL}; COMPLETE_WITH_LIST(list_FETCH1); } *** *** 1201,1216 } /* ! * Complete FETCH with "FROM" or "TO". (Is there a ! * difference? If not, remove one.) */ else if (pg_strcasecmp(prev3_wd, "FETCH") == 0 || pg_strcasecmp(prev3_wd, "MOVE") == 0) { ! static const char *const list_FROMTO[] = ! {"FROM", "TO", NULL}; ! COMPLETE_WITH_LIST(list_FROMTO); } /* GRANT && REVOKE*/ --- 1201,1217 } /* ! * Complete FETCH with "FROM" or "IN". These are ! * equivalent, but we may as well tab-complete both: perhaps some ! * users prefer one variant or the other. */ else if (pg_strcasecmp(prev3_wd, "FETCH") == 0 || pg_strcasecmp(prev3_wd, "MOVE") == 0) { ! static const char *const list_FROMIN[] = ! {"FROM", "IN", NULL}; ! COMPLETE_WITH_LIST(list_FROMIN); } /* GRANT && REVOKE*/ ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]