cursors with prepared statements
I have developed a patch that allows declaring cursors over prepared
statements:
DECLARE cursor_name CURSOR FOR prepared_statement_name
[ USING param, param, ... ]
This is an SQL standard feature. ECPG already supports it (with
different internals).
Internally, this just connects existing functionality in different ways,
so it doesn't really introduce anything new.
One point worth pondering is how to pass the parameters of the prepared
statements. The actual SQL standard syntax would be
DECLARE cursor_name CURSOR FOR prepared_statement_name;
OPEN cursor_name USING param, param;
But since we don't have the OPEN statement in direct SQL, it made sense
to me to attach the USING clause directly to the DECLARE statement.
Curiously, the direct EXECUTE statement uses the non-standard syntax
EXECUTE prep_stmt (param, param);
instead of the standard
EXECUTE prep_stmt USING param, param;
I tried to consolidate this. But using
DECLARE c CURSOR FOR p (foo, bar)
leads to parsing conflicts (and looks confusing?), and instead allowing
EXECUTE + USING leads to a mess in the ECPG parser that exhausted me.
So I'm leaving it as is for now and might give supporting EXECUTE +
USING another try later on.
When looking at the patch, some parts will look easier through git diff -w.
And the changes in the ECPG parser are needed because ECPG already
supported that syntax separately, but now it needs to override the rules
from the main parser instead. That stuff has test coverage, fortunately.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Cursors-over-prepared-statements.patchtext/plain; charset=UTF-8; name=0001-Cursors-over-prepared-statements.patch; x-mac-creator=0; x-mac-type=0Download
From c1e8ecf95599a9085e5f16bcd4aab3f13a2d6800 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Thu, 7 Jun 2018 11:46:16 -0400
Subject: [PATCH] Cursors over prepared statements
Add command variant
DECLARE cursor_name CURSOR FOR prepared_statement_name [ USING param, param, ... ]
to open a cursor over a previously defined prepared statement.
---
doc/src/sgml/ref/declare.sgml | 37 +++++++
src/backend/commands/portalcmds.c | 109 +++++++++++++++------
src/backend/commands/prepare.c | 16 ++-
src/backend/parser/analyze.c | 2 +-
src/backend/parser/gram.y | 24 +++++
src/include/commands/prepare.h | 3 +
src/interfaces/ecpg/preproc/check_rules.pl | 3 +
src/interfaces/ecpg/preproc/ecpg.addons | 63 +++++++++++-
src/interfaces/ecpg/preproc/ecpg.trailer | 65 ------------
src/interfaces/ecpg/preproc/ecpg.type | 1 -
src/interfaces/ecpg/preproc/parse.pl | 2 +
src/test/regress/expected/portals.out | 54 ++++++++++
src/test/regress/sql/portals.sql | 40 ++++++++
13 files changed, 309 insertions(+), 110 deletions(-)
diff --git a/doc/src/sgml/ref/declare.sgml b/doc/src/sgml/ref/declare.sgml
index 34ca9df243..2b127bdd6a 100644
--- a/doc/src/sgml/ref/declare.sgml
+++ b/doc/src/sgml/ref/declare.sgml
@@ -28,6 +28,9 @@
<synopsis>
DECLARE <replaceable class="parameter">name</replaceable> [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
CURSOR [ { WITH | WITHOUT } HOLD ] FOR <replaceable class="parameter">query</replaceable>
+
+DECLARE <replaceable class="parameter">name</replaceable> [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
+ CURSOR [ { WITH | WITHOUT } HOLD ] FOR <replaceable class="parameter">prepared_statement</replaceable> [ USING <replaceable class="parameter">parameter</replaceable> [, ...] ]
</synopsis>
</refsynopsisdiv>
@@ -130,6 +133,31 @@ <title>Parameters</title>
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><replaceable class="parameter">prepared_statement</replaceable></term>
+ <listitem>
+ <para>
+ The name of the prepared statement (created with <xref
+ linkend="sql-prepare"/>) which will provide the rows to be returned by
+ the cursor. The prepared statement is restricted to contain the same
+ kinds of statements as mentioned under <replaceable
+ class="parameter">query</replaceable> above.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><replaceable class="parameter">parameter</replaceable></term>
+ <listitem>
+ <para>
+ The actual value of a parameter to the prepared statement. This
+ must be an expression yielding a value that is compatible with
+ the data type of this parameter, as was determined when the
+ prepared statement was created.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
<para>
@@ -313,6 +341,14 @@ <title>Examples</title>
See <xref linkend="sql-fetch"/> for more
examples of cursor usage.
</para>
+
+ <para>
+ To declare a cursor via a prepared statement:
+<programlisting>
+PREPARE p1 AS SELECT name, price FROM produce WHERE color = $1;
+DECLARE c2 CURSOR FOR p1 USING 'green';
+</programlisting>
+ </para>
</refsect1>
<refsect1>
@@ -343,6 +379,7 @@ <title>See Also</title>
<simplelist type="inline">
<member><xref linkend="sql-close"/></member>
+ <member><xref linkend="sql-execute"/></member>
<member><xref linkend="sql-fetch"/></member>
<member><xref linkend="sql-move"/></member>
</simplelist>
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 568499761f..6c5b274b51 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -25,6 +25,7 @@
#include "access/xact.h"
#include "commands/portalcmds.h"
+#include "commands/prepare.h"
#include "executor/executor.h"
#include "executor/tstoreReceiver.h"
#include "rewrite/rewriteHandler.h"
@@ -44,9 +45,13 @@ PerformCursorOpen(DeclareCursorStmt *cstmt, ParamListInfo params,
{
Query *query = castNode(Query, cstmt->query);
List *rewritten;
- PlannedStmt *plan;
+ PlannedStmt *plan = NULL;
+ PreparedStatement *prepstmt = NULL;
+ ParamListInfo paramLI = NULL;
+ EState *estate = NULL;
Portal portal;
MemoryContext oldContext;
+ CachedPlan *cplan = NULL;
/*
* Disallow empty-string cursor name (conflicts with protocol-level
@@ -65,31 +70,61 @@ PerformCursorOpen(DeclareCursorStmt *cstmt, ParamListInfo params,
if (!(cstmt->options & CURSOR_OPT_HOLD))
RequireTransactionBlock(isTopLevel, "DECLARE CURSOR");
- /*
- * Parse analysis was done already, but we still have to run the rule
- * rewriter. We do not do AcquireRewriteLocks: we assume the query either
- * came straight from the parser, or suitable locks were acquired by
- * plancache.c.
- *
- * Because the rewriter and planner tend to scribble on the input, we make
- * a preliminary copy of the source querytree. This prevents problems in
- * the case that the DECLARE CURSOR is in a portal or plpgsql function and
- * is executed repeatedly. (See also the same hack in EXPLAIN and
- * PREPARE.) XXX FIXME someday.
- */
- rewritten = QueryRewrite((Query *) copyObject(query));
+ if (query->commandType == CMD_SELECT)
+ {
+ /*
+ * Parse analysis was done already, but we still have to run the rule
+ * rewriter. We do not do AcquireRewriteLocks: we assume the query either
+ * came straight from the parser, or suitable locks were acquired by
+ * plancache.c.
+ *
+ * Because the rewriter and planner tend to scribble on the input, we make
+ * a preliminary copy of the source querytree. This prevents problems in
+ * the case that the DECLARE CURSOR is in a portal or plpgsql function and
+ * is executed repeatedly. (See also the same hack in EXPLAIN and
+ * PREPARE.) XXX FIXME someday.
+ */
+ rewritten = QueryRewrite((Query *) copyObject(query));
- /* SELECT should never rewrite to more or less than one query */
- if (list_length(rewritten) != 1)
- elog(ERROR, "non-SELECT statement in DECLARE CURSOR");
+ /* SELECT should never rewrite to more or less than one query */
+ if (list_length(rewritten) != 1)
+ elog(ERROR, "non-SELECT statement in DECLARE CURSOR");
- query = linitial_node(Query, rewritten);
+ query = linitial_node(Query, rewritten);
- if (query->commandType != CMD_SELECT)
- elog(ERROR, "non-SELECT statement in DECLARE CURSOR");
+ if (query->commandType != CMD_SELECT)
+ elog(ERROR, "non-SELECT statement in DECLARE CURSOR");
- /* Plan the query, applying the specified options */
- plan = pg_plan_query(query, cstmt->options, params);
+ /* Plan the query, applying the specified options */
+ plan = pg_plan_query(query, cstmt->options, params);
+ }
+ else if (query->commandType == CMD_UTILITY)
+ {
+ ExecuteStmt *es = castNode(ExecuteStmt, query->utilityStmt);
+ PlannedStmt *pstmt;
+
+ prepstmt = FetchPreparedStatement(es->name, true);
+
+ if (prepstmt->plansource->num_params > 0)
+ {
+ estate = CreateExecutorState();
+ estate->es_param_list_info = params;
+ paramLI = PreparedStatementEvaluateParams(prepstmt, es->params,
+ queryString, estate);
+ }
+
+ cplan = GetCachedPlan(prepstmt->plansource, paramLI, false, NULL);
+
+ if (list_length(cplan->stmt_list) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("prepared statement is not a SELECT")));
+ pstmt = linitial_node(PlannedStmt, cplan->stmt_list);
+ if (pstmt->commandType != CMD_SELECT)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("prepared statement is not a SELECT")));
+ }
/*
* Create a portal and copy the plan and queryString into its memory.
@@ -98,16 +133,30 @@ PerformCursorOpen(DeclareCursorStmt *cstmt, ParamListInfo params,
oldContext = MemoryContextSwitchTo(portal->portalContext);
- plan = copyObject(plan);
-
queryString = pstrdup(queryString);
- PortalDefineQuery(portal,
- NULL,
- queryString,
- "SELECT", /* cursor's query is always a SELECT */
- list_make1(plan),
- NULL);
+ if (plan)
+ {
+ plan = copyObject(plan);
+
+ PortalDefineQuery(portal,
+ NULL,
+ queryString,
+ "SELECT", /* cursor's query is always a SELECT */
+ list_make1(plan),
+ NULL);
+ }
+ else
+ {
+ PortalDefineQuery(portal,
+ NULL,
+ queryString,
+ prepstmt->plansource->commandTag,
+ cplan->stmt_list,
+ cplan);
+
+ plan = linitial(cplan->stmt_list);
+ }
/*----------
* Also copy the outer portal's parameter list into the inner portal's
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index b945b1556a..a4a6626654 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -46,8 +46,6 @@
static HTAB *prepared_queries = NULL;
static void InitQueryHashTable(void);
-static ParamListInfo EvaluateParams(PreparedStatement *pstmt, List *params,
- const char *queryString, EState *estate);
static Datum build_regtype_array(Oid *param_types, int num_params);
/*
@@ -229,8 +227,8 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause,
*/
estate = CreateExecutorState();
estate->es_param_list_info = params;
- paramLI = EvaluateParams(entry, stmt->params,
- queryString, estate);
+ paramLI = PreparedStatementEvaluateParams(entry, stmt->params,
+ queryString, estate);
}
/* Create a new portal to run the query in */
@@ -312,7 +310,7 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause,
}
/*
- * EvaluateParams: evaluate a list of parameters.
+ * PreparedStatementEvaluateParams: evaluate a list of parameters.
*
* pstmt: statement we are getting parameters for.
* params: list of given parameter expressions (raw parser output!)
@@ -323,8 +321,8 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause,
* CreateQueryDesc(), which allows the executor to make use of the parameters
* during query execution.
*/
-static ParamListInfo
-EvaluateParams(PreparedStatement *pstmt, List *params,
+ParamListInfo
+PreparedStatementEvaluateParams(PreparedStatement *pstmt, List *params,
const char *queryString, EState *estate)
{
Oid *param_types = pstmt->plansource->param_types;
@@ -665,8 +663,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
*/
estate = CreateExecutorState();
estate->es_param_list_info = params;
- paramLI = EvaluateParams(entry, execstmt->params,
- queryString, estate);
+ paramLI = PreparedStatementEvaluateParams(entry, execstmt->params,
+ queryString, estate);
}
/* Replan if needed, and acquire a transient refcount */
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 05f57591e4..f6d0753dd1 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2412,7 +2412,7 @@ transformDeclareCursorStmt(ParseState *pstate, DeclareCursorStmt *stmt)
/* Grammar should not have allowed anything but SELECT */
if (!IsA(query, Query) ||
- query->commandType != CMD_SELECT)
+ (query->commandType != CMD_SELECT && query->commandType != CMD_UTILITY))
elog(ERROR, "unexpected non-SELECT command in DECLARE CURSOR");
/*
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 90dfac2cb1..2639174b26 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11125,6 +11125,30 @@ DeclareCursorStmt: DECLARE cursor_name cursor_options CURSOR opt_hold FOR Select
n->query = $7;
$$ = (Node *)n;
}
+ | DECLARE cursor_name cursor_options CURSOR opt_hold FOR name
+ {
+ DeclareCursorStmt *n = makeNode(DeclareCursorStmt);
+ ExecuteStmt *es = makeNode(ExecuteStmt);
+
+ n->portalname = $2;
+ n->options = $3 | $5 | CURSOR_OPT_FAST_PLAN;
+ es->name = $7;
+ es->params = NIL;
+ n->query = (Node *)es;
+ $$ = (Node *)n;
+ }
+ | DECLARE cursor_name cursor_options CURSOR opt_hold FOR name USING expr_list
+ {
+ DeclareCursorStmt *n = makeNode(DeclareCursorStmt);
+ ExecuteStmt *es = makeNode(ExecuteStmt);
+
+ n->portalname = $2;
+ n->options = $3 | $5 | CURSOR_OPT_FAST_PLAN;
+ es->name = $7;
+ es->params = $9;
+ n->query = (Node *)es;
+ $$ = (Node *)n;
+ }
;
cursor_name: name { $$ = $1; }
diff --git a/src/include/commands/prepare.h b/src/include/commands/prepare.h
index ffec029df4..3691170120 100644
--- a/src/include/commands/prepare.h
+++ b/src/include/commands/prepare.h
@@ -55,6 +55,9 @@ extern void DropPreparedStatement(const char *stmt_name, bool showError);
extern TupleDesc FetchPreparedStatementResultDesc(PreparedStatement *stmt);
extern List *FetchPreparedStatementTargetList(PreparedStatement *stmt);
+extern ParamListInfo PreparedStatementEvaluateParams(PreparedStatement *pstmt, List *params,
+ const char *queryString, EState *estate);
+
extern void DropAllPreparedStatements(void);
#endif /* PREPARE_H */
diff --git a/src/interfaces/ecpg/preproc/check_rules.pl b/src/interfaces/ecpg/preproc/check_rules.pl
index 6c8b004854..b0cdce7c19 100644
--- a/src/interfaces/ecpg/preproc/check_rules.pl
+++ b/src/interfaces/ecpg/preproc/check_rules.pl
@@ -36,6 +36,9 @@
}
my %replace_line = (
+ 'DeclareCursorStmtDECLAREcursor_namecursor_optionsCURSORopt_holdFORname' =>
+ 'DECLARE cursor_name cursor_options CURSOR opt_hold FOR prepared_name',
+
'ExecuteStmtEXECUTEnameexecute_param_clause' =>
'EXECUTE prepared_name execute_param_clause execute_rest',
diff --git a/src/interfaces/ecpg/preproc/ecpg.addons b/src/interfaces/ecpg/preproc/ecpg.addons
index ca3efadc48..922d1fffd2 100644
--- a/src/interfaces/ecpg/preproc/ecpg.addons
+++ b/src/interfaces/ecpg/preproc/ecpg.addons
@@ -63,10 +63,6 @@ ECPG: stmtViewStmt rule
whenever_action(2);
free($1);
}
- | ECPGCursorStmt
- {
- output_simple_statement($1);
- }
| ECPGDeallocateDescr
{
fprintf(base_yyout,"ECPGdeallocate_desc(__LINE__, %s);",$1);
@@ -334,6 +330,65 @@ ECPG: DeclareCursorStmtDECLAREcursor_namecursor_optionsCURSORopt_holdFORSelectSt
else
$$ = cat2_str(adjust_outofscope_cursor_vars(this), comment);
}
+ECPG: DeclareCursorStmtDECLAREcursor_namecursor_optionsCURSORopt_holdFORprepared_name block
+ {
+ struct cursor *ptr, *this;
+ char *cursor_marker = $2[0] == ':' ? mm_strdup("$0") : mm_strdup($2);
+ int (* strcmp_fn)(const char *, const char *) = (($2[0] == ':' || $2[0] == '"') ? strcmp : pg_strcasecmp);
+ struct variable *thisquery = (struct variable *)mm_alloc(sizeof(struct variable));
+ const char *con = connection ? connection : "NULL";
+ char *comment;
+
+ for (ptr = cur; ptr != NULL; ptr = ptr->next)
+ {
+ if (strcmp_fn($2, ptr->name) == 0)
+ {
+ /* re-definition is a bug */
+ if ($2[0] == ':')
+ mmerror(PARSE_ERROR, ET_ERROR, "using variable \"%s\" in different declare statements is not supported", $2+1);
+ else
+ mmerror(PARSE_ERROR, ET_ERROR, "cursor \"%s\" is already defined", $2);
+ }
+ }
+
+ this = (struct cursor *) mm_alloc(sizeof(struct cursor));
+
+ /* initial definition */
+ this->next = cur;
+ this->name = $2;
+ this->function = (current_function ? mm_strdup(current_function) : NULL);
+ this->connection = connection;
+ this->command = cat_str(6, mm_strdup("declare"), cursor_marker, $3, mm_strdup("cursor"), $5, mm_strdup("for $1"));
+ this->argsresult = NULL;
+ this->argsresult_oos = NULL;
+
+ thisquery->type = &ecpg_query;
+ thisquery->brace_level = 0;
+ thisquery->next = NULL;
+ thisquery->name = (char *) mm_alloc(sizeof("ECPGprepared_statement(, , __LINE__)") + strlen(con) + strlen($7));
+ sprintf(thisquery->name, "ECPGprepared_statement(%s, %s, __LINE__)", con, $7);
+
+ this->argsinsert = NULL;
+ this->argsinsert_oos = NULL;
+ if ($2[0] == ':')
+ {
+ struct variable *var = find_variable($2 + 1);
+ remove_variable_from_list(&argsinsert, var);
+ add_variable_to_head(&(this->argsinsert), var, &no_indicator);
+ }
+ add_variable_to_head(&(this->argsinsert), thisquery, &no_indicator);
+
+ cur = this;
+
+ comment = cat_str(3, mm_strdup("/*"), mm_strdup(this->command), mm_strdup("*/"));
+
+ if ((braces_open > 0) && INFORMIX_MODE) /* we're in a function */
+ $$ = cat_str(3, adjust_outofscope_cursor_vars(this),
+ mm_strdup("ECPG_informix_reset_sqlca();"),
+ comment);
+ else
+ $$ = cat2_str(adjust_outofscope_cursor_vars(this), comment);
+ }
ECPG: ClosePortalStmtCLOSEcursor_name block
{
char *cursor_marker = $2[0] == ':' ? mm_strdup("$0") : $2;
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index 19dc781885..d3123742eb 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -283,71 +283,6 @@ prepared_name: name
| char_variable { $$ = $1; }
;
-/*
- * Declare a prepared cursor. The syntax is different from the standard
- * declare statement, so we create a new rule.
- */
-ECPGCursorStmt: DECLARE cursor_name cursor_options CURSOR opt_hold FOR prepared_name
- {
- struct cursor *ptr, *this;
- char *cursor_marker = $2[0] == ':' ? mm_strdup("$0") : mm_strdup($2);
- int (* strcmp_fn)(const char *, const char *) = (($2[0] == ':' || $2[0] == '"') ? strcmp : pg_strcasecmp);
- struct variable *thisquery = (struct variable *)mm_alloc(sizeof(struct variable));
- const char *con = connection ? connection : "NULL";
- char *comment;
-
- for (ptr = cur; ptr != NULL; ptr = ptr->next)
- {
- if (strcmp_fn($2, ptr->name) == 0)
- {
- /* re-definition is a bug */
- if ($2[0] == ':')
- mmerror(PARSE_ERROR, ET_ERROR, "using variable \"%s\" in different declare statements is not supported", $2+1);
- else
- mmerror(PARSE_ERROR, ET_ERROR, "cursor \"%s\" is already defined", $2);
- }
- }
-
- this = (struct cursor *) mm_alloc(sizeof(struct cursor));
-
- /* initial definition */
- this->next = cur;
- this->name = $2;
- this->function = (current_function ? mm_strdup(current_function) : NULL);
- this->connection = connection;
- this->command = cat_str(6, mm_strdup("declare"), cursor_marker, $3, mm_strdup("cursor"), $5, mm_strdup("for $1"));
- this->argsresult = NULL;
- this->argsresult_oos = NULL;
-
- thisquery->type = &ecpg_query;
- thisquery->brace_level = 0;
- thisquery->next = NULL;
- thisquery->name = (char *) mm_alloc(sizeof("ECPGprepared_statement(, , __LINE__)") + strlen(con) + strlen($7));
- sprintf(thisquery->name, "ECPGprepared_statement(%s, %s, __LINE__)", con, $7);
-
- this->argsinsert = NULL;
- this->argsinsert_oos = NULL;
- if ($2[0] == ':')
- {
- struct variable *var = find_variable($2 + 1);
- remove_variable_from_list(&argsinsert, var);
- add_variable_to_head(&(this->argsinsert), var, &no_indicator);
- }
- add_variable_to_head(&(this->argsinsert), thisquery, &no_indicator);
-
- cur = this;
-
- comment = cat_str(3, mm_strdup("/*"), mm_strdup(this->command), mm_strdup("*/"));
-
- if ((braces_open > 0) && INFORMIX_MODE) /* we're in a function */
- $$ = cat_str(3, adjust_outofscope_cursor_vars(this),
- mm_strdup("ECPG_informix_reset_sqlca();"),
- comment);
- else
- $$ = cat2_str(adjust_outofscope_cursor_vars(this), comment);
- }
- ;
-
ECPGExecuteImmediateStmt: EXECUTE IMMEDIATE execstring
{
/* execute immediate means prepare the statement and
diff --git a/src/interfaces/ecpg/preproc/ecpg.type b/src/interfaces/ecpg/preproc/ecpg.type
index 9497b91b9d..fab5b2d73a 100644
--- a/src/interfaces/ecpg/preproc/ecpg.type
+++ b/src/interfaces/ecpg/preproc/ecpg.type
@@ -5,7 +5,6 @@
%type <str> ECPGColLabel
%type <str> ECPGColLabelCommon
%type <str> ECPGConnect
-%type <str> ECPGCursorStmt
%type <str> ECPGDeallocateDescr
%type <str> ECPGDeclaration
%type <str> ECPGDeclare
diff --git a/src/interfaces/ecpg/preproc/parse.pl b/src/interfaces/ecpg/preproc/parse.pl
index b20383ab17..695118cbda 100644
--- a/src/interfaces/ecpg/preproc/parse.pl
+++ b/src/interfaces/ecpg/preproc/parse.pl
@@ -99,6 +99,8 @@
'SHOW SESSION AUTHORIZATION ecpg_into',
'returning_clauseRETURNINGtarget_list' =>
'RETURNING target_list opt_ecpg_into',
+ 'DeclareCursorStmtDECLAREcursor_namecursor_optionsCURSORopt_holdFORname' =>
+ 'DECLARE cursor_name cursor_options CURSOR opt_hold FOR prepared_name',
'ExecuteStmtEXECUTEnameexecute_param_clause' =>
'EXECUTE prepared_name execute_param_clause execute_rest',
'ExecuteStmtCREATEOptTempTABLEcreate_as_targetASEXECUTEnameexecute_param_clause'
diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out
index 048b2fc3e3..72ffdceec7 100644
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -1376,3 +1376,57 @@ fetch backward all in c2;
(3 rows)
rollback;
+-- cursors over prepared statements
+prepare foo as select generate_series(1,3) as g;
+begin;
+declare c1 cursor for foo;
+fetch all in c1;
+ g
+---
+ 1
+ 2
+ 3
+(3 rows)
+
+rollback;
+deallocate foo;
+begin;
+declare c1 cursor for foo;
+ERROR: prepared statement "foo" does not exist
+rollback;
+prepare foo1 (int, int) as select generate_series($1, $2);
+begin;
+declare c1 cursor for foo1 using 2, 4;
+fetch all in c1;
+ generate_series
+-----------------
+ 2
+ 3
+ 4
+(3 rows)
+
+rollback;
+begin;
+declare c1 cursor for foo1 using 3, 5;
+fetch all in c1;
+ generate_series
+-----------------
+ 3
+ 4
+ 5
+(3 rows)
+
+rollback;
+begin;
+declare c1 cursor for foo1 using 'foo', 'bar';
+ERROR: invalid input syntax for integer: "foo"
+LINE 1: declare c1 cursor for foo1 using 'foo', 'bar';
+ ^
+rollback;
+CREATE TABLE cursor (a int);
+prepare foo2 as insert into cursor values ($1);
+begin;
+declare c1 cursor for foo2 using 1;
+ERROR: prepared statement is not a SELECT
+rollback;
+DROP TABLE cursor;
diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql
index d1a589094e..05891a47cf 100644
--- a/src/test/regress/sql/portals.sql
+++ b/src/test/regress/sql/portals.sql
@@ -510,3 +510,43 @@ CREATE TABLE cursor (a int);
fetch all in c2;
fetch backward all in c2;
rollback;
+
+-- cursors over prepared statements
+prepare foo as select generate_series(1,3) as g;
+
+begin;
+declare c1 cursor for foo;
+fetch all in c1;
+rollback;
+
+deallocate foo;
+
+begin;
+declare c1 cursor for foo;
+rollback;
+
+prepare foo1 (int, int) as select generate_series($1, $2);
+
+begin;
+declare c1 cursor for foo1 using 2, 4;
+fetch all in c1;
+rollback;
+
+begin;
+declare c1 cursor for foo1 using 3, 5;
+fetch all in c1;
+rollback;
+
+begin;
+declare c1 cursor for foo1 using 'foo', 'bar';
+rollback;
+
+CREATE TABLE cursor (a int);
+
+prepare foo2 as insert into cursor values ($1);
+
+begin;
+declare c1 cursor for foo2 using 1;
+rollback;
+
+DROP TABLE cursor;
base-commit: 848b1f3e358f4a1bb98d8c4a07ff8ee5fd7ea9a0
--
2.17.1
On Fri, Jun 8, 2018 at 1:12 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
I have developed a patch that allows declaring cursors over prepared
statements:DECLARE cursor_name CURSOR FOR prepared_statement_name
[ USING param, param, ... ]This is an SQL standard feature. ECPG already supports it (with
different internals).Internally, this just connects existing functionality in different ways,
so it doesn't really introduce anything new.One point worth pondering is how to pass the parameters of the prepared
statements. The actual SQL standard syntax would beDECLARE cursor_name CURSOR FOR prepared_statement_name;
OPEN cursor_name USING param, param;But since we don't have the OPEN statement in direct SQL, it made sense
to me to attach the USING clause directly to the DECLARE statement.Curiously, the direct EXECUTE statement uses the non-standard syntax
EXECUTE prep_stmt (param, param);
instead of the standard
EXECUTE prep_stmt USING param, param;
I tried to consolidate this. But using
DECLARE c CURSOR FOR p (foo, bar)
leads to parsing conflicts (and looks confusing?), and instead allowing
EXECUTE + USING leads to a mess in the ECPG parser that exhausted me.
So I'm leaving it as is for now and might give supporting EXECUTE +
USING another try later on.
Sounds like a reasonable approach. Have you not considered using a
special OPEN syntax because there are some other forms of problems
with it?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On 6/11/18 09:57, Amit Kapila wrote:
Sounds like a reasonable approach. Have you not considered using a
special OPEN syntax because there are some other forms of problems
with it?
There is no OPEN command in direct SQL. Do you mean whether I have
considered introducing an OPEN command? Yes, but it seems to me that
that would create weird inconsistencies and doesn't seem very useful in
practice.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Jun 11, 2018 at 9:56 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 6/11/18 09:57, Amit Kapila wrote:
Sounds like a reasonable approach. Have you not considered using a
special OPEN syntax because there are some other forms of problems
with it?There is no OPEN command in direct SQL. Do you mean whether I have
considered introducing an OPEN command?
Yes.
Yes, but it seems to me that
that would create weird inconsistencies and doesn't seem very useful in
practice.
Okay, if that doesn't make the job easy, then there is not much use in
pursuing that direction.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On 07/06/18 22:42, Peter Eisentraut wrote:
I have developed a patch that allows declaring cursors over prepared
statements:DECLARE cursor_name CURSOR FOR prepared_statement_name
[ USING param, param, ... ]This is an SQL standard feature. ECPG already supports it (with
different internals).Internally, this just connects existing functionality in different ways,
so it doesn't really introduce anything new.One point worth pondering is how to pass the parameters of the prepared
statements. The actual SQL standard syntax would beDECLARE cursor_name CURSOR FOR prepared_statement_name;
OPEN cursor_name USING param, param;But since we don't have the OPEN statement in direct SQL, it made sense
to me to attach the USING clause directly to the DECLARE statement.
Hmm. I'm not excited about adding PostgreSQL-extensions to the SQL
standard. It's confusing, and risks conflicting with future additions to
the standard. ECPG supports the actual standard syntax, with OPEN,
right? So this wouldn't be consistent with ECPG, either.
Curiously, the direct EXECUTE statement uses the non-standard syntax
EXECUTE prep_stmt (param, param);
instead of the standard
EXECUTE prep_stmt USING param, param;
I tried to consolidate this. But using
DECLARE c CURSOR FOR p (foo, bar)
leads to parsing conflicts (and looks confusing?),
How about
DECLARE c CURSOR FOR EXECUTE p (foo, bar)
? As a user, I'm already familiar with the "EXECUTE p (foo, bar)"
syntax, so that's what I would intuitively try to use with DECLARE as
well. In fact, I think I tried doing just that once, and was
disappointed that it didn't work.
and instead allowing
EXECUTE + USING leads to a mess in the ECPG parser that exhausted me.
So I'm leaving it as is for now and might give supporting EXECUTE +
USING another try later on.
The attached patch seems to do the trick, of allowing EXECUTE + USING.
I'm not sure this is worth the trouble, though, since EXECUTE as a plain
SQL command is a PostgreSQL-extension anyway.
This also adds a test case for the existing "EXECUTE <stmt> (<params>)"
syntax in ECPG. The current ECPG parsing of that is actually a bit
weird, it allows "EXECUTE stmt (:param1) USING :param2", which seems
unintentional. This patch rejects that syntax.
- Heikki
Attachments:
0001-Add-support-for-EXECUTE-stmt-USING-params-syntax.patchtext/x-patch; name=0001-Add-support-for-EXECUTE-stmt-USING-params-syntax.patchDownload
From 3b6cad3f6ecb615442bd0d0f365fbdec91cf9317 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed, 11 Jul 2018 19:47:43 +0300
Subject: [PATCH 1/1] Add support for EXECUTE <stmt> USING <params> syntax.
This is the SQL-standard equivalent of "EXECUTE <stmt> (<params>)".
TODO: docs.
---
src/backend/parser/gram.y | 1 +
src/interfaces/ecpg/preproc/check_rules.pl | 2 +-
src/interfaces/ecpg/preproc/ecpg.addons | 2 +-
src/interfaces/ecpg/preproc/ecpg.trailer | 9 ++++
src/interfaces/ecpg/preproc/parse.pl | 2 +-
src/interfaces/ecpg/test/expected/sql-execute.c | 51 ++++++++++++++++++----
.../ecpg/test/expected/sql-execute.stderr | 24 +++++++---
.../ecpg/test/expected/sql-execute.stdout | 1 +
src/interfaces/ecpg/test/sql/execute.pgc | 14 ++++++
src/test/regress/expected/prepare.out | 7 +++
src/test/regress/sql/prepare.sql | 3 ++
11 files changed, 100 insertions(+), 16 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 90dfac2cb1..851363fa4e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -10779,6 +10779,7 @@ ExecuteStmt: EXECUTE name execute_param_clause
;
execute_param_clause: '(' expr_list ')' { $$ = $2; }
+ | USING expr_list { $$ = $2; }
| /* EMPTY */ { $$ = NIL; }
;
diff --git a/src/interfaces/ecpg/preproc/check_rules.pl b/src/interfaces/ecpg/preproc/check_rules.pl
index 6c8b004854..ee67817be0 100644
--- a/src/interfaces/ecpg/preproc/check_rules.pl
+++ b/src/interfaces/ecpg/preproc/check_rules.pl
@@ -37,7 +37,7 @@ if ($verbose)
my %replace_line = (
'ExecuteStmtEXECUTEnameexecute_param_clause' =>
- 'EXECUTE prepared_name execute_param_clause execute_rest',
+ 'EXECUTE prepared_name execute_rest',
'ExecuteStmtCREATEOptTempTABLEcreate_as_targetASEXECUTEnameexecute_param_clause'
=> 'CREATE OptTemp TABLE create_as_target AS EXECUTE prepared_name execute_param_clause',
diff --git a/src/interfaces/ecpg/preproc/ecpg.addons b/src/interfaces/ecpg/preproc/ecpg.addons
index ca3efadc48..9606ad4783 100644
--- a/src/interfaces/ecpg/preproc/ecpg.addons
+++ b/src/interfaces/ecpg/preproc/ecpg.addons
@@ -283,7 +283,7 @@ ECPG: PrepareStmtPREPAREprepared_nameprep_type_clauseASPreparableStmt block
$$.type = NULL;
$$.stmt = $4;
}
-ECPG: ExecuteStmtEXECUTEprepared_nameexecute_param_clauseexecute_rest block
+ECPG: ExecuteStmtEXECUTEprepared_nameexecute_rest block
{ $$ = $2; }
ECPG: DeclareCursorStmtDECLAREcursor_namecursor_optionsCURSORopt_holdFORSelectStmt block
{
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index 19dc781885..22ad65c257 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -1881,7 +1881,16 @@ Iresult: Iconst { $$ = $1; }
}
;
+/*
+ * The backend grammar supports EXECUTE <stmt> USING, but in ECPG, we also
+ * support optional INTO, before or after the USING clause. This replaces the
+ * opt_execute_param_clause rule in the backend grammar.
+ *
+ * We also support the non-standard EXECUTE <stmt> (<params>) syntax. To keep
+ * things simple, any INTO clause must come after the params with that syntax.
+ */
execute_rest: /* EMPTY */ { $$ = EMPTY; }
+ | '(' using_list ')' opt_ecpg_into { $$ = EMPTY; }
| ecpg_using opt_ecpg_into { $$ = EMPTY; }
| ecpg_into ecpg_using { $$ = EMPTY; }
| ecpg_into { $$ = EMPTY; }
diff --git a/src/interfaces/ecpg/preproc/parse.pl b/src/interfaces/ecpg/preproc/parse.pl
index b20383ab17..ba556c2063 100644
--- a/src/interfaces/ecpg/preproc/parse.pl
+++ b/src/interfaces/ecpg/preproc/parse.pl
@@ -100,7 +100,7 @@ my %replace_line = (
'returning_clauseRETURNINGtarget_list' =>
'RETURNING target_list opt_ecpg_into',
'ExecuteStmtEXECUTEnameexecute_param_clause' =>
- 'EXECUTE prepared_name execute_param_clause execute_rest',
+ 'EXECUTE prepared_name execute_rest',
'ExecuteStmtCREATEOptTempTABLEcreate_as_targetASEXECUTEnameexecute_param_clause'
=> 'CREATE OptTemp TABLE create_as_target AS EXECUTE prepared_name execute_param_clause',
'PrepareStmtPREPAREnameprep_type_clauseASPreparableStmt' =>
diff --git a/src/interfaces/ecpg/test/expected/sql-execute.c b/src/interfaces/ecpg/test/expected/sql-execute.c
index cac91dc599..871cb266bd 100644
--- a/src/interfaces/ecpg/test/expected/sql-execute.c
+++ b/src/interfaces/ecpg/test/expected/sql-execute.c
@@ -302,29 +302,64 @@ if (sqlca.sqlcode < 0) sqlprint();}
printf("name[%d]=%8.8s\tamount[%d]=%d\tletter[%d]=%c\n", i, n, i, a, i, l);
}
+ /* test the non-standard syntax of passing parameters without USING */
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_execute, "f",
+ ECPGt_const,"2",(long)1,(long)1,strlen("2"),
+ ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EOIT,
+ ECPGt_char,(name),(long)8,(long)8,(8)*sizeof(char),
+ ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L,
+ ECPGt_int,(amount),(long)1,(long)8,sizeof(int),
+ ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L,
+ ECPGt_char,(letter),(long)1,(long)8,(1)*sizeof(char),
+ ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EORT);
+#line 108 "execute.pgc"
+
+if (sqlca.sqlcode < 0) sqlprint();}
+#line 108 "execute.pgc"
+
+
+ for (i=0, j=sqlca.sqlerrd[2]; i<j; i++)
+ {
+ /* exec sql begin declare section */
+
+
+
+#line 113 "execute.pgc"
+ char n [ 8 ] , l = letter [ i ] [ 0 ] ;
+
+#line 114 "execute.pgc"
+ int a = amount [ i ] ;
+/* exec sql end declare section */
+#line 115 "execute.pgc"
+
+
+ strncpy(n, name[i], 8);
+ printf("name[%d]=%8.8s\tamount[%d]=%d\tletter[%d]=%c\n", i, n, i, a, i, l);
+ }
+
{ ECPGdeallocate(__LINE__, 0, NULL, "f");
-#line 107 "execute.pgc"
+#line 121 "execute.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 107 "execute.pgc"
+#line 121 "execute.pgc"
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "drop table test", ECPGt_EOIT, ECPGt_EORT);
-#line 108 "execute.pgc"
+#line 122 "execute.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 108 "execute.pgc"
+#line 122 "execute.pgc"
{ ECPGtrans(__LINE__, NULL, "commit");
-#line 109 "execute.pgc"
+#line 123 "execute.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 109 "execute.pgc"
+#line 123 "execute.pgc"
{ ECPGdisconnect(__LINE__, "CURRENT");
-#line 110 "execute.pgc"
+#line 124 "execute.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 110 "execute.pgc"
+#line 124 "execute.pgc"
return 0;
diff --git a/src/interfaces/ecpg/test/expected/sql-execute.stderr b/src/interfaces/ecpg/test/expected/sql-execute.stderr
index 96b46bd158..f8eae9b61e 100644
--- a/src/interfaces/ecpg/test/expected/sql-execute.stderr
+++ b/src/interfaces/ecpg/test/expected/sql-execute.stderr
@@ -156,15 +156,29 @@
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ecpg_get_data on line 94: RESULT: t offset: -1; array: no
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: deallocate_one on line 107: name f
+[NO_PID]: ecpg_execute on line 108: query: select * from test where amount = $1; with 1 parameter(s) on connection main
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 108: query: drop table test; with 0 parameter(s) on connection main
+[NO_PID]: ecpg_execute on line 108: using PQexecPrepared for "select * from test where amount = $1"
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 108: using PQexec
+[NO_PID]: ecpg_free_params on line 108: parameter 1 = 2
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_process_output on line 108: OK: DROP TABLE
+[NO_PID]: ecpg_process_output on line 108: correctly got 1 tuples with 3 fields
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ECPGtrans on line 109: action "commit"; connection "main"
+[NO_PID]: ecpg_get_data on line 108: RESULT: db: 'r1' offset: -1; array: no
+[NO_PID]: sqlca: code: 0, state: 00000
+[NO_PID]: ecpg_get_data on line 108: RESULT: 2 offset: -1; array: no
+[NO_PID]: sqlca: code: 0, state: 00000
+[NO_PID]: ecpg_get_data on line 108: RESULT: t offset: -1; array: no
+[NO_PID]: sqlca: code: 0, state: 00000
+[NO_PID]: deallocate_one on line 121: name f
+[NO_PID]: sqlca: code: 0, state: 00000
+[NO_PID]: ecpg_execute on line 122: query: drop table test; with 0 parameter(s) on connection main
+[NO_PID]: sqlca: code: 0, state: 00000
+[NO_PID]: ecpg_execute on line 122: using PQexec
+[NO_PID]: sqlca: code: 0, state: 00000
+[NO_PID]: ecpg_process_output on line 122: OK: DROP TABLE
+[NO_PID]: sqlca: code: 0, state: 00000
+[NO_PID]: ECPGtrans on line 123: action "commit"; connection "main"
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: deallocate_one on line 0: name i
[NO_PID]: sqlca: code: 0, state: 00000
diff --git a/src/interfaces/ecpg/test/expected/sql-execute.stdout b/src/interfaces/ecpg/test/expected/sql-execute.stdout
index 5f9295ae4c..3b2c9f37eb 100644
--- a/src/interfaces/ecpg/test/expected/sql-execute.stdout
+++ b/src/interfaces/ecpg/test/expected/sql-execute.stdout
@@ -10,3 +10,4 @@ name[6]=db: 'r1' amount[6]=111 letter[6]=f
name[7]=db: 'r1' amount[7]=112 letter[7]=t
name[0]=db: 'r1' amount[0]=1 letter[0]=f
name[0]=db: 'r1' amount[0]=2 letter[0]=t
+name[0]=db: 'r1' amount[0]=2 letter[0]=t
diff --git a/src/interfaces/ecpg/test/sql/execute.pgc b/src/interfaces/ecpg/test/sql/execute.pgc
index cc9814e9be..f272836e1e 100644
--- a/src/interfaces/ecpg/test/sql/execute.pgc
+++ b/src/interfaces/ecpg/test/sql/execute.pgc
@@ -104,6 +104,20 @@ exec sql end declare section;
printf("name[%d]=%8.8s\tamount[%d]=%d\tletter[%d]=%c\n", i, n, i, a, i, l);
}
+ /* test the non-standard syntax of passing parameters without USING */
+ exec sql execute f (2) into :name, :amount, :letter;
+
+ for (i=0, j=sqlca.sqlerrd[2]; i<j; i++)
+ {
+ exec sql begin declare section;
+ char n[8], l = letter[i][0];
+ int a = amount[i];
+ exec sql end declare section;
+
+ strncpy(n, name[i], 8);
+ printf("name[%d]=%8.8s\tamount[%d]=%d\tletter[%d]=%c\n", i, n, i, a, i, l);
+ }
+
exec sql deallocate f;
exec sql drop table test;
exec sql commit;
diff --git a/src/test/regress/expected/prepare.out b/src/test/regress/expected/prepare.out
index 7016e82bd4..eda94fd525 100644
--- a/src/test/regress/expected/prepare.out
+++ b/src/test/regress/expected/prepare.out
@@ -64,6 +64,13 @@ EXECUTE q2('postgres');
postgres | f | t
(1 row)
+-- the SQL standard way of passing parameters, with USING
+EXECUTE q2 USING 'postgres';
+ datname | datistemplate | datallowconn
+----------+---------------+--------------
+ postgres | f | t
+(1 row)
+
PREPARE q3(text, int, float, boolean, oid, smallint) AS
SELECT * FROM tenk1 WHERE string4 = $1 AND (four = $2 OR
ten = $3::bigint OR true = $4 OR oid = $5 OR odd = $6::int)
diff --git a/src/test/regress/sql/prepare.sql b/src/test/regress/sql/prepare.sql
index 25f814b466..85803e60a4 100644
--- a/src/test/regress/sql/prepare.sql
+++ b/src/test/regress/sql/prepare.sql
@@ -36,6 +36,9 @@ PREPARE q2(text) AS
EXECUTE q2('postgres');
+-- the SQL standard way of passing parameters, with USING
+EXECUTE q2 USING 'postgres';
+
PREPARE q3(text, int, float, boolean, oid, smallint) AS
SELECT * FROM tenk1 WHERE string4 = $1 AND (four = $2 OR
ten = $3::bigint OR true = $4 OR oid = $5 OR odd = $6::int)
--
2.11.0
On 11.07.18 19:07, Heikki Linnakangas wrote:
One point worth pondering is how to pass the parameters of the prepared
statements. The actual SQL standard syntax would beDECLARE cursor_name CURSOR FOR prepared_statement_name;
OPEN cursor_name USING param, param;But since we don't have the OPEN statement in direct SQL, it made sense
to me to attach the USING clause directly to the DECLARE statement.Hmm. I'm not excited about adding PostgreSQL-extensions to the SQL
standard.
Isn't that what we do all the time?
It's confusing, and risks conflicting with future additions to
the standard. ECPG supports the actual standard syntax, with OPEN,
right? So this wouldn't be consistent with ECPG, either.
It would be consistent for the case of no parameters.
Curiously, the direct EXECUTE statement uses the non-standard syntax
EXECUTE prep_stmt (param, param);
instead of the standard
EXECUTE prep_stmt USING param, param;
I tried to consolidate this. But using
DECLARE c CURSOR FOR p (foo, bar)
leads to parsing conflicts (and looks confusing?),
How about
DECLARE c CURSOR FOR EXECUTE p (foo, bar)
That's not the standard syntax for the case of no parameters.
The attached patch seems to do the trick, of allowing EXECUTE + USING.
I'm not sure this is worth the trouble, though, since EXECUTE as a plain
SQL command is a PostgreSQL-extension anyway.
I think it's a PostgreSQL extension that we allow just about anything to
be executed directly. So we should still use the standard syntax either
way. It would be weird if EXECUTE or any other command had different
syntax in direct SQL, ECPG, PL/pgSQL, etc. We have some differences
already, but we shouldn't create more.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Jul 16, 2018 at 8:56 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
The attached patch seems to do the trick, of allowing EXECUTE + USING.
I'm not sure this is worth the trouble, though, since EXECUTE as a plain
SQL command is a PostgreSQL-extension anyway.I think it's a PostgreSQL extension that we allow just about anything to
be executed directly. So we should still use the standard syntax either
way. It would be weird if EXECUTE or any other command had different
syntax in direct SQL, ECPG, PL/pgSQL, etc. We have some differences
already, but we shouldn't create more.
Hmm. Your proposal to attach a USING clause to DECLARE .. CURSOR FOR
rather than inventing an OPEN command is an argument for a PostgreSQL
syntax extension, but your proposal to write DECLARE .. CURSOR FOR
rather than DECLARE .. CURSOR FOR EXECUTE is an argument for standard
syntax over and against a PostgreSQL extension.
That sounds a little contradictory, but I think I agree with it. If
we allow a USING clause for DECLARE .. CURSOR FOR, that doesn't
prevent somebody from inventing an OPEN command in the future. As
part of introducing such an OPEN command, DECLARE .. CURSOR FOR could
be made not to fail if the prepared statement has parameters but no
USING commands. On the other hand, if we insist on injecting the word
EXECUTE into the syntax as Heikki proposes, that's purely and simply
an incompatibility with the standard's syntax, as well as with what
ECPG already does.
So +1 for your position.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 16/07/18 15:56, Peter Eisentraut wrote:
On 11.07.18 19:07, Heikki Linnakangas wrote:
It's confusing, and risks conflicting with future additions to
the standard. ECPG supports the actual standard syntax, with OPEN,
right? So this wouldn't be consistent with ECPG, either.It would be consistent for the case of no parameters.
True. Except that if I understand correctly, in the standard syntax you
still need to use OPEN after the DECLARE CURSOR, even when there are no
parameters.
Curiously, the direct EXECUTE statement uses the non-standard syntax
EXECUTE prep_stmt (param, param);
instead of the standard
EXECUTE prep_stmt USING param, param;
I tried to consolidate this. But using
DECLARE c CURSOR FOR p (foo, bar)
leads to parsing conflicts (and looks confusing?),
How about
DECLARE c CURSOR FOR EXECUTE p (foo, bar)
That's not the standard syntax for the case of no parameters.
My thinking here is that "DECLARE c CURSOR FOR <statement>" is standard
syntax. And we already have "EXECUTE p (foo, bar)" as a form of
statement, along with "SELECT ...", "EXPLAIN ..." and so forth. Allowing
"DECLARE c CURSOR FOR EXECUTE p (foo, bar)" would not introduce a new
syntax, it would just allow the existing two commands, DECLARE CURSOR,
and EXECUTE, to be used together.
- Heikki
On Wed, Jul 18, 2018 at 10:27 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 16/07/18 15:56, Peter Eisentraut wrote:
On 11.07.18 19:07, Heikki Linnakangas wrote:
It's confusing, and risks conflicting with future additions to
the standard. ECPG supports the actual standard syntax, with OPEN,
right? So this wouldn't be consistent with ECPG, either.It would be consistent for the case of no parameters.
True. Except that if I understand correctly, in the standard syntax you
still need to use OPEN after the DECLARE CURSOR, even when there are no
parameters.Curiously, the direct EXECUTE statement uses the non-standard syntax
EXECUTE prep_stmt (param, param);
instead of the standard
EXECUTE prep_stmt USING param, param;
I tried to consolidate this. But using
DECLARE c CURSOR FOR p (foo, bar)
leads to parsing conflicts (and looks confusing?),
How about
DECLARE c CURSOR FOR EXECUTE p (foo, bar)
That's not the standard syntax for the case of no parameters.
My thinking here is that "DECLARE c CURSOR FOR <statement>" is standard
syntax. And we already have "EXECUTE p (foo, bar)" as a form of
statement, along with "SELECT ...", "EXPLAIN ..." and so forth. Allowing
"DECLARE c CURSOR FOR EXECUTE p (foo, bar)" would not introduce a new
syntax, it would just allow the existing two commands, DECLARE CURSOR,
and EXECUTE, to be used together.
This patch went through the last few commitfests without any noticeable
activity. Both suggested patches are still good (can be applied and passed all
the tests, except the minor text mismatch in the original one), but looks like
the discussion stopped right in the middle. Are there any more opinions about
OPEN vs DECLARE .. CURSOR FOR here or any other plans about the patch?
On Thu, Nov 22, 2018 at 11:11 AM Dmitry Dolgov <9erthalion6@gmail.com> wrote:
On Wed, Jul 18, 2018 at 10:27 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 16/07/18 15:56, Peter Eisentraut wrote:
On 11.07.18 19:07, Heikki Linnakangas wrote:
It's confusing, and risks conflicting with future additions to
the standard. ECPG supports the actual standard syntax, with OPEN,
right? So this wouldn't be consistent with ECPG, either.It would be consistent for the case of no parameters.
True. Except that if I understand correctly, in the standard syntax you
still need to use OPEN after the DECLARE CURSOR, even when there are no
parameters.Curiously, the direct EXECUTE statement uses the non-standard syntax
EXECUTE prep_stmt (param, param);
instead of the standard
EXECUTE prep_stmt USING param, param;
I tried to consolidate this. But using
DECLARE c CURSOR FOR p (foo, bar)
leads to parsing conflicts (and looks confusing?),
How about
DECLARE c CURSOR FOR EXECUTE p (foo, bar)
That's not the standard syntax for the case of no parameters.
My thinking here is that "DECLARE c CURSOR FOR <statement>" is standard
syntax. And we already have "EXECUTE p (foo, bar)" as a form of
statement, along with "SELECT ...", "EXPLAIN ..." and so forth. Allowing
"DECLARE c CURSOR FOR EXECUTE p (foo, bar)" would not introduce a new
syntax, it would just allow the existing two commands, DECLARE CURSOR,
and EXECUTE, to be used together.This patch went through the last few commitfests without any noticeable
activity. Both suggested patches are still good (can be applied and passed all
the tests, except the minor text mismatch in the original one), but looks like
the discussion stopped right in the middle. Are there any more opinions about
OPEN vs DECLARE .. CURSOR FOR here or any other plans about the patch?
I hope it's not another abandoned patch, but due to lack of response I'm
marking it as returned with feedback. If someone didn't see my previous
inquiry and ready to continue working on this patch - feel free to change it
back.