Re: 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(dot)linnakangas(at)enterprisedb(dot)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 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?
>

+1 - There are similar problems with recordsets

> --
>  Heikki Linnakangas
>  EnterpriseDB   http://www.enterprisedb.com
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)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(dot)linnakangas(at)enterprisedb(dot)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?
>
>

In a more complex example you might use $1 in more than one place in the
query.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)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(at)dunslane(dot)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

On 06/08/10 01:13, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)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

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)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(dot)linnakangas(at)enterprisedb(dot)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 statement
>
> The 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(at)postgresql(dot)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(dot)villemain(dot)debian(at)gmail(dot)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(dot)linnakangas(at)enterprisedb(dot)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(at)sss(dot)pgh(dot)pa(dot)us>:
> =?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric(dot)villemain(dot)debian(at)gmail(dot)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(dot)villemain(dot)debian(at)gmail(dot)com> writes:
> 2010/8/16 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> =?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric(dot)villemain(dot)debian(at)gmail(dot)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(at)sss(dot)pgh(dot)pa(dot)us>:
> =?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric(dot)villemain(dot)debian(at)gmail(dot)com> writes:
>> 2010/8/16 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>> =?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric(dot)villemain(dot)debian(at)gmail(dot)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(dot)villemain(dot)debian(at)gmail(dot)com>:
> 2010/8/17 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> =?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric(dot)villemain(dot)debian(at)gmail(dot)com> writes:
>>> 2010/8/16 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>>> =?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric(dot)villemain(dot)debian(at)gmail(dot)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

=?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric(dot)villemain(dot)debian(at)gmail(dot)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(at)sss(dot)pgh(dot)pa(dot)us>:
> =?ISO-8859-1?Q?C=E9dric_Villemain?= <cedric(dot)villemain(dot)debian(at)gmail(dot)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.

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