Horton
Menu

Thread

  1. pgsql: Coerce 'unknown' type parameters to the right type in the

    Heikki Linnakangas <heikki@postgresql.org> — 2010-08-18T12:20:15Z

    Log Message:
-----------
Coerce 'unknown' type parameters to the right type in the fixed-params
parse_analyze() function. That case occurs e.g with PL/pgSQL
EXECUTE ... USING 'stringconstant'.

The coercion with a CoerceViaIO node. The result is similar to the coercion
via input function performed for unknown constants in coerce_type(),
except that this happens at runtime.

Backpatch to 9.0. The issue is present in 8.4 as well, but the coerce param
hook infrastructure this patch relies on was introduced in 9.0. Given the
lack of user reports and harmlessness of the bug, it's not worth attempting
a different fix just for 8.4.

Modified Files:
--------------
    pgsql/src/backend/parser:
        parse_param.c (r2.4 -> r2.5)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/parser/parse_param.c?r1=2.4&r2=2.5)

  2. Re: pgsql: Coerce 'unknown' type parameters to the right type in the

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-08-18T13:57:19Z

    heikki@postgresql.org (Heikki Linnakangas) writes:
> Log Message:
> -----------
> Coerce 'unknown' type parameters to the right type in the fixed-params
> parse_analyze() function. That case occurs e.g with PL/pgSQL
> EXECUTE ... USING 'stringconstant'.

> The coercion with a CoerceViaIO node. The result is similar to the coercion
> via input function performed for unknown constants in coerce_type(),
> except that this happens at runtime.

Unfortunately, this entirely fails to enforce the rule that an unknown
Param be coerced the same way everywhere.  You'd need a cleanup pass as
well, cf check_variable_parameters().

			regards, tom lane

  3. Re: pgsql: Coerce 'unknown' type parameters to the right type in the

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-08-18T15:03:34Z

    On 18/08/10 16:57, Tom Lane wrote:
> heikki@postgresql.org (Heikki Linnakangas) writes:
>> Log Message:
>> -----------
>> Coerce 'unknown' type parameters to the right type in the fixed-params
>> parse_analyze() function. That case occurs e.g with PL/pgSQL
>> EXECUTE ... USING 'stringconstant'.
>
>> The coercion with a CoerceViaIO node. The result is similar to the coercion
>> via input function performed for unknown constants in coerce_type(),
>> except that this happens at runtime.
>
> Unfortunately, this entirely fails to enforce the rule that an unknown
> Param be coerced the same way everywhere.  You'd need a cleanup pass as
> well, cf check_variable_parameters().

Yeah, you're right. I'll find a way to do the cleanup pass in fixed 
params case too.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

  4. Re: [COMMITTERS] pgsql: Coerce 'unknown' type parameters to the right type in the

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-08-19T08:22:36Z

    On 18/08/10 18:03, Heikki Linnakangas wrote:
> On 18/08/10 16:57, Tom Lane wrote:
>> heikki@postgresql.org (Heikki Linnakangas) writes:
>>> Log Message:
>>> -----------
>>> Coerce 'unknown' type parameters to the right type in the fixed-params
>>> parse_analyze() function. That case occurs e.g with PL/pgSQL
>>> EXECUTE ... USING 'stringconstant'.
>>
>>> The coercion with a CoerceViaIO node. The result is similar to the
>>> coercion
>>> via input function performed for unknown constants in coerce_type(),
>>> except that this happens at runtime.
>>
>> Unfortunately, this entirely fails to enforce the rule that an unknown
>> Param be coerced the same way everywhere. You'd need a cleanup pass as
>> well, cf check_variable_parameters().
>
> Yeah, you're right. I'll find a way to do the cleanup pass in fixed
> params case too.

It turned out to be messier than I imagined, but I have a working patch 
now. It still doesn't behave exactly like the variable params case, 
though. To wit:

postgres=# DO $$
declare
   t text;
begin
   EXECUTE 'SELECT 1+ $1, $1' INTO t USING '123' ;
   RAISE NOTICE '%', t;
end;
$$;
ERROR:  could not determine data type of parameter $1
LINE 1: SELECT 1+ $1, $1
                       ^
QUERY:  SELECT 1+ $1, $1
CONTEXT:  PL/pgSQL function "inline_code_block" line 5 at EXECUTE statement

The varparams code doesn't thrown an error on that. At the first 
reference to $1, the parameter is resolved to int4. On the 2nd 
reference, it's an int4 and there's nothing to force coercion to 
anything else, so it's returned as an int4. In the fixed params case, 
however, that throws an error. We could make it behave the same if we 
really wanted to, but that would add even more code.

I'm starting to wonder if it's worth enforcing the rule that all unknown 
Params must be coerced to the same target type. We could just document 
the behavior. Or maybe we should revert the whole thing, and add a check 
to PL/pgSQL EXECUTE USING code to just throw a nicer error message if 
you pass an unknown parameter in the USING clause.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

  5. Re: Re: [COMMITTERS] pgsql: Coerce 'unknown' type parameters to the right type in the

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-08-19T13:47:17Z

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> I'm starting to wonder if it's worth enforcing the rule that all unknown 
> Params must be coerced to the same target type. We could just document 
> the behavior. Or maybe we should revert the whole thing, and add a check 
> to PL/pgSQL EXECUTE USING code to just throw a nicer error message if 
> you pass an unknown parameter in the USING clause.

+1 for the latter.  There's no good reason to be passing unknowns to USING.
I'm also getting more and more uncomfortable with the amount of new
behavior that's being pushed into an existing SPI call.

Another possibility is for EXECUTE USING to coerce any unknowns to TEXT
before it calls the parser at all.  This would square with the typical
default assumption for unknown literals, and it would avoid having to
have any semantics changes below the SPI call.

			regards, tom lane

  6. Re: Re: [COMMITTERS] pgsql: Coerce 'unknown' type parameters to the right type in the

    Robert Haas <robertmhaas@gmail.com> — 2010-08-19T15:08:25Z

    On Thu, Aug 19, 2010 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Another possibility is for EXECUTE USING to coerce any unknowns to TEXT
> before it calls the parser at all.  This would square with the typical
> default assumption for unknown literals, and it would avoid having to
> have any semantics changes below the SPI call.

That seems more intuitive than just chucking an error.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

  7. Re: Re: [COMMITTERS] pgsql: Coerce 'unknown' type parameters to the right type in the

    David E. Wheeler <david@kineticode.com> — 2010-08-19T15:09:36Z

    On Aug 19, 2010, at 8:08 AM, Robert Haas wrote:

>> Another possibility is for EXECUTE USING to coerce any unknowns to TEXT
>> before it calls the parser at all.  This would square with the typical
>> default assumption for unknown literals, and it would avoid having to
>> have any semantics changes below the SPI call.
> 
> That seems more intuitive than just chucking an error.

It'd be nice if SPI itself could work this way for UNKNOWNs, too.

Best,

David

  8. Re: Re: [COMMITTERS] pgsql: Coerce 'unknown' type parameters to the right type in the

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-08-19T16:56:54Z

    On 19/08/10 18:08, Robert Haas wrote:
> On Thu, Aug 19, 2010 at 9:47 AM, Tom Lane<tgl@sss.pgh.pa.us>  wrote:
>> Another possibility is for EXECUTE USING to coerce any unknowns to TEXT
>> before it calls the parser at all.  This would square with the typical
>> default assumption for unknown literals, and it would avoid having to
>> have any semantics changes below the SPI call.
>
> That seems more intuitive than just chucking an error.

Ok, I reverted the previous patch, and did that instead.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com