Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
> Did this ever get applied? If so, I can't find it. No, my bad, I simply forgot about it, sorry. Will work on this now. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at googlemail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Boszormenyi Zoltan wrote: > Kevin Grittner ?rta: > > Michael Meskes wrote: > > > >> All prior ECPG versions were fine because dynamic cursor names > >> were only added in 9.0. Apparently only this one place was > >> missed. So this is a bug in the new feature, however not such a > >> major one that it warrants the complete removal IMO. I'd prefer to > >> fix this in 9.0.1. > >> > > As we are so late in the beta phase, we can live with that, hopefully > you will find time by then to review the patch which is actually not > that complex, only a bit large. The part of ECPGdo() that deals with > auto-preparing statements is moved closer to calling ecpg_execute(), > after the varargs are converted to stmt->inlist and ->outlist. > > >> Hope this cleans it up a bit. > >> > > > > Thanks. I think I get it now. > > > > To restate from another angle, to confirm my understanding: UPDATE > > WHERE CURRENT OF is working for cursors with the name hard-coded in > > the embedded statement, which is the only way cursor names were > > allowed to be specified prior to 9.0; 9.0 implements dynamic cursor > > names, allowing you to use a variable for the cursor name; but this > > one use of a cursor name isn't allowing a variable yet. Do I have > > it right? (If so, I now see why it would be considered a bug.) > > > > Yes, you understand it correctly. Did this ever get applied? If so, I can't find it. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Boszormenyi Zoltan writes: > Alvaro Herrera Ãrta: >> Since we're still in the beta phase, it makes sense to apply the fix >> right now so that it appears in 9.0. No point in waiting for 9.0.1. > It boils down to the fact that Michael doesn't have too much time > and no one else knows ECPG in depth. So... Yeah. I think what Michael is saying is he doesn't have time to review the patch now and doesn't want to hold up 9.0 until he does. We can delay 9.0 for him, or apply the patch unreviewed, or allow 9.0 to go out with this as a known bug. I don't much care for #2, given the size of the patch. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Alvaro Herrera írta: > Excerpts from Michael Meskes's message of jue ago 05 05:39:46 -0400 2010: > >> Sorry I thought Zoltan's explanation was clear enough. All prior ECPG >> versions were fine because dynamic cursor names were only added in 9.0. >> Apparently only this one place was missed. So this is a bug in the new >> feature, however not such a major one that it warrants the complete removal >> IMO. I'd prefer to fix this in 9.0.1. >> > > Since we're still in the beta phase, it makes sense to apply the fix > right now so that it appears in 9.0. No point in waiting for 9.0.1. > It boils down to the fact that Michael doesn't have too much time and no one else knows ECPG in depth. So... -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Excerpts from Michael Meskes's message of jue ago 05 05:39:46 -0400 2010: > Sorry I thought Zoltan's explanation was clear enough. All prior ECPG > versions were fine because dynamic cursor names were only added in 9.0. > Apparently only this one place was missed. So this is a bug in the new > feature, however not such a major one that it warrants the complete removal > IMO. I'd prefer to fix this in 9.0.1. Since we're still in the beta phase, it makes sense to apply the fix right now so that it appears in 9.0. No point in waiting for 9.0.1. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Sorry I thought Zoltan's explanation was clear enough. All prior ECPG versions were fine because dynamic cursor names were only added in 9.0. Apparently only this one place was missed. So this is a bug in the new feature, however not such a major one that it warrants the complete removal IMO. I'd prefer to fix this in 9.0.1. Hope this cleans it up a bit. Michael "Kevin Grittner" schrieb: >Michael Meskes wrote: > >> I'd consider this a bug. > >Could you explain why? The assertions that people consider it a bug >without explanation of *why* is confusing for me. > >It sounds more like a feature of the ECPG interface that people >would really like, and which has been technically possible since >PostgreSQL 8.3, but for which nobody submitted a patch until this >week. There was some hint that a 9.0 ECPG patch added new features >which might make people expect this feature to have also been added. >If this patch isn't necessarily correct, and would be dangerous to >apply at this point, should the other patch be reverted as something >which shouldn't go out without this feature? > >-Kevin -- Sent from my Android phone with K-9 Mail. Please excuse my brevity. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Kevin Grittner írta: > Michael Meskes wrote: > >> All prior ECPG versions were fine because dynamic cursor names >> were only added in 9.0. Apparently only this one place was >> missed. So this is a bug in the new feature, however not such a >> major one that it warrants the complete removal IMO. I'd prefer to >> fix this in 9.0.1. >> As we are so late in the beta phase, we can live with that, hopefully you will find time by then to review the patch which is actually not that complex, only a bit large. The part of ECPGdo() that deals with auto-preparing statements is moved closer to calling ecpg_execute(), after the varargs are converted to stmt->inlist and ->outlist. >> Hope this cleans it up a bit. >> > > Thanks. I think I get it now. > > To restate from another angle, to confirm my understanding: UPDATE > WHERE CURRENT OF is working for cursors with the name hard-coded in > the embedded statement, which is the only way cursor names were > allowed to be specified prior to 9.0; 9.0 implements dynamic cursor > names, allowing you to use a variable for the cursor name; but this > one use of a cursor name isn't allowing a variable yet. Do I have > it right? (If so, I now see why it would be considered a bug.) > Yes, you understand it correctly. Best regards, Zoltán Böszörményi -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Michael Meskes wrote: > All prior ECPG versions were fine because dynamic cursor names > were only added in 9.0. Apparently only this one place was > missed. So this is a bug in the new feature, however not such a > major one that it warrants the complete removal IMO. I'd prefer to > fix this in 9.0.1. > > Hope this cleans it up a bit. Thanks. I think I get it now. To restate from another angle, to confirm my understanding: UPDATE WHERE CURRENT OF is working for cursors with the name hard-coded in the embedded statement, which is the only way cursor names were allowed to be specified prior to 9.0; 9.0 implements dynamic cursor names, allowing you to use a variable for the cursor name; but this one use of a cursor name isn't allowing a variable yet. Do I have it right? (If so, I now see why it would be considered a bug.) -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Michael Meskes wrote: > I'd consider this a bug. Could you explain why? The assertions that people consider it a bug without explanation of *why* is confusing for me. It sounds more like a feature of the ECPG interface that people would really like, and which has been technically possible since PostgreSQL 8.3, but for which nobody submitted a patch until this week. There was some hint that a 9.0 ECPG patch added new features which might make people expect this feature to have also been added. If this patch isn't necessarily correct, and would be dangerous to apply at this point, should the other patch be reverted as something which shouldn't go out without this feature? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Kevin Grittner írta: > Boszormenyi Zoltan wrote: > > >> attached is a patch that adds the missing feature >> > > >> I certainly feel that this should be applied to 9.0 as a bugfix. >> > > Those two statements seem to contradict one another. PostgreSQL 8.3 or so added WHERE CURRENT OF at the SQL level. 9.0 added ECPG's dynamic cursorname (a char variable carries the actual cursor name) but the WHERE CURRENT OF part was not converted, which was definitely an oversight. Whether this is a "missing feature" or a "bugfix", it's only a difference in points of view. I think this patch belongs to 9.0. > Is there some > bug manifestation beyond an unimplemented feature this fixes? > Without this, is there some regression going from 8.4 to 9.0? > I haven't looked at 8.4's regression tests but pgtypeslib/numeric.c in 8.4.4 has the same problem, so the second patch (at least the numeric.c part) can be applied there as well, maybe in even older branches. Best regards, Zoltán Böszörményi -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Boszormenyi Zoltan wrote: > attached is a patch that adds the missing feature > I certainly feel that this should be applied to 9.0 as a bugfix. Those two statements seem to contradict one another. Is there some bug manifestation beyond an unimplemented feature this fixes? Without this, is there some regression going from 8.4 to 9.0? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname
Hi,
attached is a patch that adds the missing feature to use
"WHERE CURRENT OF :curname" in UPDATE and
DELETE statements via ECPG. I used the current CVS MAIN
but also applies almost cleanly to 9.0beta4. I certainly feel that
this should be applied to 9.0 as a bugfix.
The execute.c changes were required because
1. The statement
UPDATE table SET fld1 = :input1
WHERE CURRENT OF :curname
RETURNING id + :input2;
is transformed into
UPDATE table SET fld1 = $1
WHERE CURRENT OF $0
RETURNING id + $2;
and the $0 is past $1. The current code cannot deal with such
a messed up order, and scanning the original query twice is
needed, once for $0 substitution, once for mapping $1, etc. to
the other input variables.
2. With such a statement and auto-prepare turned on, I always got
SQL error: there is no parameter $0 on line X
It turned out that the statement was prepared by the auto-prepare
machinery
before the $0 substitution. PostgreSQL allows
PREPARE mystmt AS UPDATE ... WHERE CURRENT OF mycur
even if "mycur" is currently unknown to the system. It's resolved upon
executing the prepared query, so we should allow it in ECPG even with
dynamic cursorname.
The code survives "make check" and I also went through all the regression
tests manually to check them with valgrind to see that there's no leak.
As a result, another patch is attached that fixes two memory leaks in
PGTYPESnumeric_from_asc() and PGTYPESnumeric_to_long() and
quite some leaks in the regression tests themselves.
Best regards,
Zoltán Böszörményi
diff -dcrpN pgsql.orig/src/backend/parser/gram.y pgsql-wherecurrentof/src/backend/parser/gram.y
*** pgsql.orig/src/backend/parser/gram.y 2010-07-26 10:05:46.0 +0200
--- pgsql-wherecurrentof/src/backend/parser/gram.y 2010-08-03 10:45:41.0 +0200
*** where_clause:
*** 8201,8207
/* variant for UPDATE and DELETE */
where_or_current_clause:
WHERE a_expr { $$ = $2; }
! | WHERE CURRENT_P OF name
{
CurrentOfExpr *n = makeNode(CurrentOfExpr);
/* cvarno is filled in by parse analysis */
--- 8201,8207
/* variant for UPDATE and DELETE */
where_or_current_clause:
WHERE a_expr { $$ = $2; }
! | WHERE CURRENT_P OF cursor_name
{
CurrentOfExpr *n = makeNode(CurrentOfExpr);
/* cvarno is filled in by parse analysis */
diff -dcrpN pgsql.orig/src/interfaces/ecpg/ecpglib/execute.c pgsql-wherecurrentof/src/interfaces/ecpg/ecpglib/execute.c
*** pgsql.orig/src/interfaces/ecpg/ecpglib/execute.c 2010-07-11 11:15:00.0 +0200
--- pgsql-wherecurrentof/src/interfaces/ecpg/ecpglib/execute.c 2010-08-03 16:50:43.0 +0200
*** ecpg_store_input(const int lineno, const
*** 1082,1099
return true;
}
! static void
! free_params(const char **paramValues, int nParams, bool print, int lineno)
{
int n;
! for (n = 0; n < nParams; n++)
{
if (print)
! ecpg_log("free_params on line %d: parameter %d = %s\n", lineno, n + 1, paramValues[n] ? paramValues[n] : "null");
! ecpg_free((void *) (paramValues[n]));
}
! ecpg_free(paramValues);
}
--- 1082,1109
return true;
}
! void
! ecpg_free_params(struct statement *stmt, bool print, int lineno)
{
int n;
! for (n = 0; n < stmt->nparams; n++)
{
if (print)
! ecpg_log("free_params on line %d: parameter %d = %s\n", lineno, n + 1, stmt->param_values[n] ? stmt->param_values[n] : "null");
! ecpg_free((void *) (stmt->param_values[n]));
}
! ecpg_free(stmt->param_values);
!
! stmt->nparams = 0;
! stmt->param_values = NULL;
!
! for (n = 0; n < stmt->ndollarzero; n++)
! ecpg_free((void *) (stmt->dollarzero[n]));
! ecpg_free(stmt->dollarzero);
!
! stmt->ndollarzero = 0;
! stmt->dollarzero = NULL;
}
*** insert_tobeinserted(int position, int ph
*** 1130,1135
--- 1140,1203
}
static bool
+ ecpg_replace_inline_params(struct statement * stmt)
+ {
+ struct variable *var;
+ int position = 0;
+
+ /*
+ * We have to check the $0 inline parameters first, they can appear
+ * after $1, e.g. in this example:
+ * EXEC SQL UPDATE table SET f1 = :in1 WHERE CURRENT OF :curname RETURNING id + :in2;
+ * transformed statement is:
+ * "update table set f1 = $1 WHERE CURRENT OF $0 RETURNING id + $2"
+ */
+ var = stmt->inlist;
+ while (var)
+ {
+ char *tobeinserted;
+
+ tobeinserted = NULL;
+
+ if ((position = next_insert(stmt->command, position, stmt->questionmarks) + 1) == 0)
+ break;
+
+ /*
+ * if the placeholder is '$0' we have to replace it on the client side
+ * this is for places we want to support variables at that are not
+ * supported in the backend
+ */
+ if (stmt->command[position] == '0')
+ {
+ const char **dollarzero;
+
+ if (!ecpg_store_input(stmt->lineno, stmt->force_indicator, var, &tobeinserted, false))
+ return false;
+
+
