PL/pgSQL EXECUTE '..' USING with unknown
There's a little problem with EXECUTE USING when the parameters are of
type unknown (going back to 8.4 where EXECUTE USING was introduced):
do $$
BEGIN
EXECUTE 'SELECT to_date($1, $2)' USING '17-DEC-80', 'DD-MON-YY';
END;
$$;
ERROR: failed to find conversion function from unknown to text
CONTEXT: SQL statement "SELECT to_date($1, $2)"
PL/pgSQL function "inline_code_block" line 2 at EXECUTE statement
The corresponding case works fine when used with PREPARE/EXECUTE:
postgres=# PREPARE foostmt AS SELECT to_date($1, $2);
PREPARE
postgres=# EXECUTE foostmt ('17-DEC-80', 'DD-MON-YY');
to_date
------------
1980-12-17
(1 row)
With PREPARE/EXECUTE, the query is analyzed with
parse_analyze_varparams() which allows unknown param types to be deduced
from the context. Seems we should use that for EXECUTE USING as well,
but there's no SPI interface for that.
Thoughts? Should we add an SPI_prepare_varparams() function and use that?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Hello
2010/8/5 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:
There's a little problem with EXECUTE USING when the parameters are of type
unknown (going back to 8.4 where EXECUTE USING was introduced):do $$
BEGIN
EXECUTE 'SELECT to_date($1, $2)' USING '17-DEC-80', 'DD-MON-YY';
END;
$$;
ERROR: failed to find conversion function from unknown to text
CONTEXT: SQL statement "SELECT to_date($1, $2)"
PL/pgSQL function "inline_code_block" line 2 at EXECUTE statementThe corresponding case works fine when used with PREPARE/EXECUTE:
postgres=# PREPARE foostmt AS SELECT to_date($1, $2);
PREPARE
postgres=# EXECUTE foostmt ('17-DEC-80', 'DD-MON-YY');
to_date
------------
1980-12-17
(1 row)With PREPARE/EXECUTE, the query is analyzed with parse_analyze_varparams()
which allows unknown param types to be deduced from the context. Seems we
should use that for EXECUTE USING as well, but there's no SPI interface for
that.Thoughts? Should we add an SPI_prepare_varparams() function and use that?
+1 - There are similar problems with recordsets
Show quoted text
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
There's a little problem with EXECUTE USING when the parameters are of
type unknown (going back to 8.4 where EXECUTE USING was introduced):
do $$
BEGIN
EXECUTE 'SELECT to_date($1, $2)' USING '17-DEC-80', 'DD-MON-YY';
END;
$$;
ERROR: failed to find conversion function from unknown to text
This example doesn't seem terribly compelling. Why would you bother
using USING with constants?
regards, tom lane
On 08/05/2010 05:11 PM, Tom Lane wrote:
Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> writes:
There's a little problem with EXECUTE USING when the parameters are of
type unknown (going back to 8.4 where EXECUTE USING was introduced):
do $$
BEGIN
EXECUTE 'SELECT to_date($1, $2)' USING '17-DEC-80', 'DD-MON-YY';
END;
$$;
ERROR: failed to find conversion function from unknown to textThis example doesn't seem terribly compelling. Why would you bother
using USING with constants?
In a more complex example you might use $1 in more than one place in the
query.
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
On 08/05/2010 05:11 PM, Tom Lane wrote:
This example doesn't seem terribly compelling. Why would you bother
using USING with constants?
In a more complex example you might use $1 in more than one place in the
query.
Well, that's better than no justification, but it's still pretty weak.
A bigger problem is that doing anything like this will require reversing
the logical path of causation in EXECUTE USING. Right now, we evaluate
the USING expressions first, and then their types feed forward into
parsing the EXECUTE string. What Heikki is suggesting requires
reversing that, at least to some extent. I'm not convinced it's
possible without breaking other cases that are more important.
regards, tom lane
On 06/08/10 01:13, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
On 08/05/2010 05:11 PM, Tom Lane wrote:
This example doesn't seem terribly compelling. Why would you bother
using USING with constants?In a more complex example you might use $1 in more than one place in the
query.Well, that's better than no justification, but it's still pretty weak.
A bigger problem is that doing anything like this will require reversing
the logical path of causation in EXECUTE USING. Right now, we evaluate
the USING expressions first, and then their types feed forward into
parsing the EXECUTE string. What Heikki is suggesting requires
reversing that, at least to some extent. I'm not convinced it's
possible without breaking other cases that are more important.
One approach is to handle the conversion from unknown to the right data
type transparently in the backend. Attached patch adds a
coerce-param-hook for fixed params that returns a CoerceViaIO node to
convert the param to the right type at runtime. That's quite similar to
the way unknown constants are handled.
The patch doesn't currently check that a parameter is only resolved to
one type in the same query, but that can be added.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Attachments:
fixed-params-unknown-coerce-hook-1.patchtext/x-diff; name=fixed-params-unknown-coerce-hook-1.patchDownload
diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c
index 20abf00..f4381fc 100644
--- a/src/backend/parser/parse_param.c
+++ b/src/backend/parser/parse_param.c
@@ -55,6 +55,9 @@ static Node *variable_paramref_hook(ParseState *pstate, ParamRef *pref);
static Node *variable_coerce_param_hook(ParseState *pstate, Param *param,
Oid targetTypeId, int32 targetTypeMod,
int location);
+static Node *fixed_coerce_param_hook(ParseState *pstate, Param *param,
+ Oid targetTypeId, int32 targetTypeMod,
+ int location);
static bool check_parameter_resolution_walker(Node *node, ParseState *pstate);
@@ -71,7 +74,7 @@ parse_fixed_parameters(ParseState *pstate,
parstate->numParams = numParams;
pstate->p_ref_hook_state = (void *) parstate;
pstate->p_paramref_hook = fixed_paramref_hook;
- /* no need to use p_coerce_param_hook */
+ pstate->p_coerce_param_hook = fixed_coerce_param_hook;
}
/*
@@ -170,6 +173,43 @@ variable_paramref_hook(ParseState *pstate, ParamRef *pref)
return (Node *) param;
}
+static Node *
+fixed_coerce_param_hook(ParseState *pstate, Param *param,
+ Oid targetTypeId, int32 targetTypeMode,
+ int location)
+{
+ if (param->paramkind == PARAM_EXTERN && param->paramtype == UNKNOWNOID)
+ {
+ /*
+ * Input is a Param of previously undetermined type, and we want to
+ * update our knowledge of the Param's type.
+ */
+ FixedParamState *parstate = (FixedParamState *) pstate->p_ref_hook_state;
+ int paramno = param->paramid;
+ CoerceViaIO *iocoerce;
+
+ if (paramno <= 0 || /* shouldn't happen, but... */
+ paramno > parstate->numParams)
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_PARAMETER),
+ errmsg("there is no parameter $%d", paramno),
+ parser_errposition(pstate, param->location)));
+
+ /* Build a CoerceViaIO node */
+ iocoerce = makeNode(CoerceViaIO);
+ iocoerce->arg = (Expr *) param;
+ iocoerce->resulttype = targetTypeId;
+ iocoerce->coerceformat = COERCE_IMPLICIT_CAST;
+ iocoerce->location = location;
+
+ return (Node *) iocoerce;
+ }
+
+ /* Else signal to proceed with normal coercion */
+ return NULL;
+}
+
+
/*
* Coerce a Param to a query-requested datatype, in the varparams case.
*/
On 06/08/10 01:13, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
On 08/05/2010 05:11 PM, Tom Lane wrote:
This example doesn't seem terribly compelling. Why would you bother
using USING with constants?In a more complex example you might use $1 in more than one place in the
query.Well, that's better than no justification, but it's still pretty weak.
A bigger problem is that doing anything like this will require reversing
the logical path of causation in EXECUTE USING. Right now, we evaluate
the USING expressions first, and then their types feed forward into
parsing the EXECUTE string. What Heikki is suggesting requires
reversing that, at least to some extent. I'm not convinced it's
possible without breaking other cases that are more important.
One approach is to handle the conversion from unknown to the right data
type transparently in the backend. Attached patch adds a
coerce-param-hook for fixed params that returns a CoerceViaIO node to
convert the param to the right type at runtime. That's quite similar to
the way unknown constants are handled.
The patch doesn't currently check that a parameter is only resolved to
one type in the same query, but that can be added.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Attachments:
fixed-params-unknown-coerce-hook-1.patchtext/x-diff; name=fixed-params-unknown-coerce-hook-1.patchDownload
diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c
index 20abf00..f4381fc 100644
--- a/src/backend/parser/parse_param.c
+++ b/src/backend/parser/parse_param.c
@@ -55,6 +55,9 @@ static Node *variable_paramref_hook(ParseState *pstate, ParamRef *pref);
static Node *variable_coerce_param_hook(ParseState *pstate, Param *param,
Oid targetTypeId, int32 targetTypeMod,
int location);
+static Node *fixed_coerce_param_hook(ParseState *pstate, Param *param,
+ Oid targetTypeId, int32 targetTypeMod,
+ int location);
static bool check_parameter_resolution_walker(Node *node, ParseState *pstate);
@@ -71,7 +74,7 @@ parse_fixed_parameters(ParseState *pstate,
parstate->numParams = numParams;
pstate->p_ref_hook_state = (void *) parstate;
pstate->p_paramref_hook = fixed_paramref_hook;
- /* no need to use p_coerce_param_hook */
+ pstate->p_coerce_param_hook = fixed_coerce_param_hook;
}
/*
@@ -170,6 +173,43 @@ variable_paramref_hook(ParseState *pstate, ParamRef *pref)
return (Node *) param;
}
+static Node *
+fixed_coerce_param_hook(ParseState *pstate, Param *param,
+ Oid targetTypeId, int32 targetTypeMode,
+ int location)
+{
+ if (param->paramkind == PARAM_EXTERN && param->paramtype == UNKNOWNOID)
+ {
+ /*
+ * Input is a Param of previously undetermined type, and we want to
+ * update our knowledge of the Param's type.
+ */
+ FixedParamState *parstate = (FixedParamState *) pstate->p_ref_hook_state;
+ int paramno = param->paramid;
+ CoerceViaIO *iocoerce;
+
+ if (paramno <= 0 || /* shouldn't happen, but... */
+ paramno > parstate->numParams)
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_PARAMETER),
+ errmsg("there is no parameter $%d", paramno),
+ parser_errposition(pstate, param->location)));
+
+ /* Build a CoerceViaIO node */
+ iocoerce = makeNode(CoerceViaIO);
+ iocoerce->arg = (Expr *) param;
+ iocoerce->resulttype = targetTypeId;
+ iocoerce->coerceformat = COERCE_IMPLICIT_CAST;
+ iocoerce->location = location;
+
+ return (Node *) iocoerce;
+ }
+
+ /* Else signal to proceed with normal coercion */
+ return NULL;
+}
+
+
/*
* Coerce a Param to a query-requested datatype, in the varparams case.
*/
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
One approach is to handle the conversion from unknown to the right data
type transparently in the backend. Attached patch adds a
coerce-param-hook for fixed params that returns a CoerceViaIO node to
convert the param to the right type at runtime. That's quite similar to
the way unknown constants are handled.
The idea of using a coerce_hook instead of inventing several new API
layers is attractive, but have you checked that there are no callers
for which this would be a bad idea?
Another issue is that this fails to mimic the usual varparams behavior
that a Param of unknown type should be resolved to only one type when it
is referenced in multiple places. I'm not sure that that's a critical
behavior, but I'm definitely not sure that it's not.
regards, tom lane
2010/8/5 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:
There's a little problem with EXECUTE USING when the parameters are of type
unknown (going back to 8.4 where EXECUTE USING was introduced):do $$
BEGIN
EXECUTE 'SELECT to_date($1, $2)' USING '17-DEC-80', 'DD-MON-YY';
END;
$$;
ERROR: failed to find conversion function from unknown to text
CONTEXT: SQL statement "SELECT to_date($1, $2)"
PL/pgSQL function "inline_code_block" line 2 at EXECUTE statementThe corresponding case works fine when used with PREPARE/EXECUTE:
Yes, and you point out another thing. EXECUTE is a way to bypass the
named prepare statement, to be sure query is replanned each time.
Unfortunely the current implementation of EXECUTE USING is not working
this way. If I read correctly, the internal cursor receive parameters
and is similar to a named prepare in the plan it produce.
I am in favor to have a complete replan for EXECUTE USING, or at least
change the docs:
http://www.postgresql.org/docs/current/static/plpgsql-statements.html#PLPGSQL-STATEMENTS-EXECUTING-DYN
«An EXECUTE with a simple constant command string and some USING
parameters, as in the first example above, is functionally equivalent
to just writing the command directly in PL/pgSQL and allowing
replacement of PL/pgSQL variables to happen automatically. The
important difference is that EXECUTE will re-plan the command on each
execution, generating a plan that is specific to the current parameter
values; whereas PL/pgSQL normally creates a generic plan and caches it
for re-use. In situations where the best plan depends strongly on the
parameter values, EXECUTE can be significantly faster; while when the
plan is not sensitive to parameter values, re-planning will be a
waste.»
postgres=# PREPARE foostmt AS SELECT to_date($1, $2);
PREPARE
postgres=# EXECUTE foostmt ('17-DEC-80', 'DD-MON-YY');
to_date
------------
1980-12-17
(1 row)With PREPARE/EXECUTE, the query is analyzed with parse_analyze_varparams()
which allows unknown param types to be deduced from the context. Seems we
should use that for EXECUTE USING as well, but there's no SPI interface for
that.Thoughts? Should we add an SPI_prepare_varparams() function and use that?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Cédric Villemain 2ndQuadrant
http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
Yes, and you point out another thing. EXECUTE is a way to bypass the
named prepare statement, to be sure query is replanned each time.
Unfortunely the current implementation of EXECUTE USING is not working
this way.
Uh ... what do you base that statement on?
regards, tom lane
On 16/08/10 03:35, Tom Lane wrote:
Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> writes:
One approach is to handle the conversion from unknown to the right data
type transparently in the backend. Attached patch adds a
coerce-param-hook for fixed params that returns a CoerceViaIO node to
convert the param to the right type at runtime. That's quite similar to
the way unknown constants are handled.The idea of using a coerce_hook instead of inventing several new API
layers is attractive, but have you checked that there are no callers
for which this would be a bad idea?
That code is used in a lot of different contexts, but I can't see any
where this could cause a problem. In general, I can't think of a case
where we would want to throw an error on an unknown parameter where we
accept an unknown constant at the same location. Completely rejecting
unknown parameters might make sense in some contexts, but that's not the
current behavior either, unknown parameters are accepted in some contexts.
Another issue is that this fails to mimic the usual varparams behavior
that a Param of unknown type should be resolved to only one type when it
is referenced in multiple places. I'm not sure that that's a critical
behavior, but I'm definitely not sure that it's not.
Yeah, that's exactly what I was referring to when I said:
The patch doesn't currently check that a parameter is only resolved to one type in the same query, but that can be added.
I'll add that check. Better to be conservative and relax it later if
needed, than to be lenient now and regret it later.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
2010/8/16 Tom Lane <tgl@sss.pgh.pa.us>:
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
Yes, and you point out another thing. EXECUTE is a way to bypass the
named prepare statement, to be sure query is replanned each time.
Unfortunely the current implementation of EXECUTE USING is not working
this way.Uh ... what do you base that statement on?
About the planning behavior ?
With USING, I get a seqscan (cost and long), without USING I have an
indexscan(short and costless).
And the pg_stat* views confirm that the index is not used.
I think that the relevant code is in exec_dynquery_with_params(...).
The SPI_cursor_open_with_args receive a complete string, or a string +
params.
My use case is very simple:
EXECUTE 'SELECT status FROM ' || l_partname::regclass
|| ' WHERE uid = ' || quote_literal(p_uid)
INTO r.flag;
vs
EXECUTE 'SELECT status FROM ' || l_partname::regclass
|| ' WHERE uid = $1'
USING p_uid
INTO r.flag;
(here it is not bad, but I was very happy to be able to do a safe uid
= ANY ($1), with p_uid an array in another context.)
regards, tom lane
--
Cédric Villemain 2ndQuadrant
http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
2010/8/16 Tom Lane <tgl@sss.pgh.pa.us>:
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
Unfortunely the current implementation of EXECUTE USING is not working
this way.Uh ... what do you base that statement on?
About the planning behavior ?
With USING, I get a seqscan (cost and long), without USING I have an
indexscan(short and costless).
It works as expected for me. What PG version are you using exactly?
Could you provide a self-contained example?
regards, tom lane
2010/8/17 Tom Lane <tgl@sss.pgh.pa.us>:
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
2010/8/16 Tom Lane <tgl@sss.pgh.pa.us>:
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
Unfortunely the current implementation of EXECUTE USING is not working
this way.Uh ... what do you base that statement on?
About the planning behavior ?
With USING, I get a seqscan (cost and long), without USING I have an
indexscan(short and costless).It works as expected for me. What PG version are you using exactly?
Could you provide a self-contained example?
postgresql 8.4.4. Yes I'll work one out this evening.
more or less : table foo (uid char(32) PK, flag boolean), uids are
md5sum. +-6M rows.
--
Cédric Villemain 2ndQuadrant
http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support
2010/8/17 Cédric Villemain <cedric.villemain.debian@gmail.com>:
2010/8/17 Tom Lane <tgl@sss.pgh.pa.us>:
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
2010/8/16 Tom Lane <tgl@sss.pgh.pa.us>:
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
Unfortunely the current implementation of EXECUTE USING is not working
this way.Uh ... what do you base that statement on?
About the planning behavior ?
With USING, I get a seqscan (cost and long), without USING I have an
indexscan(short and costless).It works as expected for me. What PG version are you using exactly?
Could you provide a self-contained example?postgresql 8.4.4. Yes I'll work one out this evening.
more or less : table foo (uid char(32) PK, flag boolean), uids are
md5sum. +-6M rows.
Here we are. A simple usecase.
--
Cédric Villemain 2ndQuadrant
http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support
Attachments:
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
Here we are. A simple usecase.
The reason you have an issue here is that the column is char(n) while
the parameter is text. So the non-USING execute is equivalent to
regression=# explain SELECT flag FROM foo where uid = 'cfcd208495d565ef66e7dff9f98764da';
QUERY PLAN
--------------------------------------------------------------------
Index Scan using foo_pkey on foo (cost=0.00..8.27 rows=1 width=1)
Index Cond: (uid = 'cfcd208495d565ef66e7dff9f98764da'::bpchar)
(2 rows)
while the EXECUTE USING is equivalent to
regression=# explain SELECT flag FROM foo where uid = 'cfcd208495d565ef66e7dff9f98764da'::text;
QUERY PLAN
--------------------------------------------------------------------
Seq Scan on foo (cost=0.00..24.02 rows=5 width=1)
Filter: ((uid)::text = 'cfcd208495d565ef66e7dff9f98764da'::text)
(2 rows)
and the reason you don't get an indexscan on the latter is that it's a
TEXT comparison not a BPCHAR comparison; which is different because of
the rules about ignoring trailing blanks.
char(n) sucks. Avoid it if possible. If you insist on using it,
be very very careful about which comparison semantics you're asking for.
regards, tom lane
2010/8/17 Tom Lane <tgl@sss.pgh.pa.us>:
=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric.villemain.debian@gmail.com> writes:
Here we are. A simple usecase.
The reason you have an issue here is that the column is char(n) while
the parameter is text. So the non-USING execute is equivalent toregression=# explain SELECT flag FROM foo where uid = 'cfcd208495d565ef66e7dff9f98764da';
QUERY PLAN
--------------------------------------------------------------------
Index Scan using foo_pkey on foo (cost=0.00..8.27 rows=1 width=1)
Index Cond: (uid = 'cfcd208495d565ef66e7dff9f98764da'::bpchar)
(2 rows)while the EXECUTE USING is equivalent to
regression=# explain SELECT flag FROM foo where uid = 'cfcd208495d565ef66e7dff9f98764da'::text;
QUERY PLAN
--------------------------------------------------------------------
Seq Scan on foo (cost=0.00..24.02 rows=5 width=1)
Filter: ((uid)::text = 'cfcd208495d565ef66e7dff9f98764da'::text)
(2 rows)and the reason you don't get an indexscan on the latter is that it's a
TEXT comparison not a BPCHAR comparison; which is different because of
the rules about ignoring trailing blanks.char(n) sucks. Avoid it if possible. If you insist on using it,
be very very careful about which comparison semantics you're asking for.
Oh! Thank you very much for those clarifications.
... and I am sorry for the noisy report ...
--
Cédric Villemain 2ndQuadrant
http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support