Re: stringtype=unspecified is null check problem - Mailing list pgsql-jdbc
| From | David G. Johnston |
|---|---|
| Subject | Re: stringtype=unspecified is null check problem |
| Date | |
| Msg-id | CAKFQuwatG9xxLm2bF+9-6LcvmhFqPFtszTDt5S+UG8Ljnfw7vw@mail.gmail.com Whole thread Raw |
| In response to | Re: stringtype=unspecified is null check problem (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
AW: stringtype=unspecified is null check problem
(Martin Handsteiner <martin.handsteiner@sibvisions.com>)
|
| List | pgsql-jdbc |
On Wed, Jan 11, 2023 at 8:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Yes, the non-determinism of the above (i.e., reversing the order of the
> tests removes the error), which implies the error is not sufficiently
> delayed to give other parts of the statement a chance to provide context,
> is also annoying. Which I suppose is why you are saying a second pass
> would be needed to get that delay in a minimally-invasive way.
Actually ... looking closer at the relevant code, there already *is*
two-pass processing. We're just using it to throw an error though.
I wonder if we can get away with retroactively changing the types
of Params that didn't get resolved from local context, along the
lines of the attached. The main issues I can see with this are:
* Is there any case where we'd copy UNKNOWNOID as the type of a
parse node just above an unresolved Param? I can't think of a
reason to do that offhand, because any such context would force
identification of the Param's type; but maybe I'm missing something.
If that did happen, this code would not fix the type of the upper
parse node. But maybe that wouldn't matter anyway??
* There is a very gross hack "for JDBC compatibility" right
adjacent to this:
/*
* If the argument is of type void and it's procedure call, interpret it
* as unknown. This allows the JDBC driver to not have to distinguish
* function and procedure calls. See also another component of this hack
* in ParseFuncOrColumn().
*/
if (*pptype == VOIDOID && pstate->p_expr_kind == EXPR_KIND_CALL_ARGUMENT)
*pptype = UNKNOWNOID;
I'm not sure if this change breaks that, and have no idea how to test.
Is the name "check_parameter_resolution_walker" open to change, we are no longer simply checking here. coerce_unresolved_parameters_hook?
I get the paranoia test shouldn't pass due to testing in variable_coerce_param_hook, but the errors produced there and here are for the same condition and probably should match. Or maybe change this one to an assert?
Re: JDBC, it would have been nice to have tests already in place when the code had gone in...hopefully the JDBC's project test suite will cover their six.
Being unable to "PREPARE ... AS CALL proc(...);" does make testing this a bit trickier. The functional version is just:
create or replace function callfunc(OUT val integer) returns integer as $$ select 1::integer; $$ language sql;
prepare cf (void) as select callfunc($1);
execute cf('text');
Which is still intact. I'm not too concerned here myself. I also did:
create or replace function echo(in val text) returns text as $$ select val; $$ language sql;
create or replace function echo(in val integer) returns integer as $$ select val; $$ language sql;
prepare cecho (void) as select echo($1);
ERROR: function echo() does not exist
LINE 1: prepare cecho (void) as select echo($1);
A function signature ignores output arguments and missing input arguments are going to be a problem in their own right. A function call has to assign concrete output types when it is defined so in theory any function call arguments are going to be known and the unknown assignment in the hack already is guaranteed to find a context data type to resolve to so this failsafe code should never be reached for those void+argument parameters.
If the above reasoning is sound, though, maybe modify the resolvedType test added to check_parameter_resolution_walker to only consider non-EXPR_KIND_CALL_ARGUMENT placed parameters.
David J.
pgsql-jdbc by date: