public inbox for [email protected]
help / color / mirror / Atom feedstringtype=unspecified is null check problem
14+ messages / 4 participants
[nested] [flat]
* stringtype=unspecified is null check problem
@ 2023-01-10 17:28 Martin Handsteiner <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Martin Handsteiner @ 2023-01-10 17:28 UTC (permalink / raw)
To: [email protected] <[email protected]>
If a bind parameter is checked for null, ERROR: could not determine data type of parameter $1 occurs.
Simple testcase:
Connection conn = DriverManager.getConnection("jdbc:postgresql://192.168.1.201:5432/testdb?stringtype=unspecified", "test", "test");
PreparedStatement stat = conn.prepareStatement("select 1 where ? is null");
stat.setNull(1, Types.VARCHAR);
stat.executeQuery();
-> Exception
For checking the parameter for null, the data type should not be relevant. At least TEXT or VARCHAR could be implicitly used.
There is a mailing post, that says, that stat.setNull(1, Types.VARCHAR); would strict set the value as VARCHAR.
https://www.postgresql.org/message-id/Pine.BSO.4.64.0702141439480.24142%40leary.csoft.net
But this seams not to happen in my case.
I have also testet to bind the value with:
stat.setNull(1, Types.NULL); or stat.setNull(1, Types.OTHER); or stat.setString(1, null);
Nothing works in this case.
As far as I understand, the jdbc driver sends the value without type information to the database. The database throws the exception.
So maybe this is a database problem.
The database can assume, that in case of ? is null it doesn't matter, which type the sent null is. So even a UNKOWN null is null or not null.
Strange is, that the following statement will work. So in this case null is unknown, but mapped.
conn.prepareStatement("select 1 where coalesce(?, null) is null");
^ permalink raw reply [nested|flat] 14+ messages in thread
* stringtype=unspecified is null check problem
@ 2023-01-10 17:44 David G. Johnston <[email protected]>
parent: Martin Handsteiner <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: David G. Johnston @ 2023-01-10 17:44 UTC (permalink / raw)
To: Martin Handsteiner <[email protected]>; +Cc: [email protected] <[email protected]>
On Tuesday, January 10, 2023, Martin Handsteiner <martin.handsteiner@
sibvisions.com> wrote:
> If a bind parameter is checked for null, ERROR: could not determine data
> type of parameter $1 occurs.
> Simple testcase:
> Connection conn = DriverManager.getConnection("jdbc:postgresql://
> 192.168.1.201:5432/testdb?stringtype=unspecified", "test", "test");
> PreparedStatement stat = conn.prepareStatement("select 1 where ? is null");
> stat.setNull(1, Types.VARCHAR);
> stat.executeQuery();
> -> Exception
>
> For checking the parameter for null, the data type should not be relevant.
> At least TEXT or VARCHAR could be implicitly used.
>
> There is a mailing post, that says, that stat.setNull(1, Types.VARCHAR);
> would strict set the value as VARCHAR.
> https://www.postgresql.org/message-id/Pine.BSO.4.64.07021414
> 39480.24142%40leary.csoft.net
> But this seams not to happen in my case.
>
> I have also testet to bind the value with:
> stat.setNull(1, Types.NULL); or stat.setNull(1, Types.OTHER); or
> stat.setString(1, null);
> Nothing works in this case.
>
> As far as I understand, the jdbc driver sends the value without type
> information to the database. The database throws the exception.
> So maybe this is a database problem.
> The database can assume, that in case of ? is null it doesn't matter,
> which type the sent null is. So even a UNKOWN null is null or not null.
>
> Strange is, that the following statement will work. So in this case null
> is unknown, but mapped.
> conn.prepareStatement("select 1 where coalesce(?, null) is null");
>
>
This is indeed how all of this works in the current design. I suggest you
add a cast to the input parameter in the query. Or choose a different
value for stringtype…
David J.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: stringtype=unspecified is null check problem
@ 2023-01-10 18:53 Martin Handsteiner <[email protected]>
parent: David G. Johnston <[email protected]>
1 sibling, 0 replies; 14+ messages in thread
From: Martin Handsteiner @ 2023-01-10 18:53 UTC (permalink / raw)
To: [email protected] <[email protected]>
The problem is about generic queries, where the user writes something like:
… where (:SEARCH is null or table.id<http://or table.id; = :SEARCH)
The parameters are replaced by ? and the value is bound to the statement.
The stringtype=unspecified solves the problem for table.id<http://for table.id; = :SEARCH, but causes the problem for :SEARCH is null.
So with stringtype=VARCHAR :SEARCH is null is working, but table.id<http://but table.id; = :SEARCH fails.
A developer is of course able to write a cast(? as varchar), normal users maybe not, and it breaks database independency.
The database parses the statement and knowns therefore the use case. It would be the central place. Maybe there is also a solution in the jdbc driver?
Statement parsing, is complex, so it would be fine, to find any solution without parsing the statement.
Am 10. Jan. 2023, um 18:44, "David G. Johnston" <[email protected]<mailto:[email protected]>> schrieb:
On Tuesday, January 10, 2023, Martin Handsteiner <[email protected]<mailto:[email protected]>> wrote:
If a bind parameter is checked for null, ERROR: could not determine data type of parameter $1 occurs.
Simple testcase:
Connection conn = DriverManager.getConnection("jdbc:postgresql://192.168.1.201:5432/testdb?stringtype=unspecified<http://192.168.1.201:5432/testdb?stringtype=unspecified>";, "test", "test");
PreparedStatement stat = conn.prepareStatement("select 1 where ? is null");
stat.setNull(1, Types.VARCHAR);
stat.executeQuery();
-> Exception
For checking the parameter for null, the data type should not be relevant. At least TEXT or VARCHAR could be implicitly used.
There is a mailing post, that says, that stat.setNull(1, Types.VARCHAR); would strict set the value as VARCHAR.
https://www.postgresql.org/message-id/Pine.BSO.4.64.0702141439480.24142%40leary.csoft.net
But this seams not to happen in my case.
I have also testet to bind the value with:
stat.setNull(1, Types.NULL); or stat.setNull(1, Types.OTHER); or stat.setString(1, null);
Nothing works in this case.
As far as I understand, the jdbc driver sends the value without type information to the database. The database throws the exception.
So maybe this is a database problem.
The database can assume, that in case of ? is null it doesn't matter, which type the sent null is. So even a UNKOWN null is null or not null.
Strange is, that the following statement will work. So in this case null is unknown, but mapped.
conn.prepareStatement("select 1 where coalesce(?, null) is null");
This is indeed how all of this works in the current design. I suggest you add a cast to the input parameter in the query. Or choose a different value for stringtype…
David J.
^ permalink raw reply [nested|flat] 14+ messages in thread
* AW: stringtype=unspecified is null check problem
@ 2023-01-11 09:48 Martin Handsteiner <[email protected]>
parent: David G. Johnston <[email protected]>
1 sibling, 2 replies; 14+ messages in thread
From: Martin Handsteiner @ 2023-01-11 09:48 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: [email protected] <[email protected]>
There are 3 use cases, where I would need one setting, that always ensures, that null can be bound…
(setNull(1, <setting>) and stringtype=<setting>)
select 1 where 1=? -- setNull(1, Types.VARCHAR) and stringtype=unspecified
select 1 where 'A'=? -- setNull(1, Types.VARCHAR) and stringtype doesn’t matter
select 1 where ? is null -- setNull(1, Types.VARCHAR) and stringtype=VARCHAR
That there is no way to binding null in a simple way is a bug for me, because the following will work, and the database has also to decide, how to map null:
select 1 where 1=null -- now the database converts null to a number
select 1 where 'A'=null -- now the database converts null to a varchar
select 1 where null is null – now the database doesn’t care… so everthing works.
The question is, what is the difference between the two, and why should the caller parse the statement, if he uses jdbc. These examples are easy, but in real world, in 1=? the 1 could also be a sub select (select max(id) from …)
Von: David G. Johnston <[email protected]>
Gesendet: Dienstag, 10. Jänner 2023 18:44
An: Martin Handsteiner <[email protected]>
Cc: [email protected]
Betreff: stringtype=unspecified is null check problem
On Tuesday, January 10, 2023, Martin Handsteiner <[email protected]<mailto:[email protected]>> wrote:
If a bind parameter is checked for null, ERROR: could not determine data type of parameter $1 occurs.
Simple testcase:
Connection conn = DriverManager.getConnection("jdbc:postgresql://192.168.1.201:5432/testdb?stringtype=unspecified<http://192.168.1.201:5432/testdb?stringtype=unspecified>";, "test", "test");
PreparedStatement stat = conn.prepareStatement("select 1 where ? is null");
stat.setNull(1, Types.VARCHAR);
stat.executeQuery();
-> Exception
For checking the parameter for null, the data type should not be relevant. At least TEXT or VARCHAR could be implicitly used.
There is a mailing post, that says, that stat.setNull(1, Types.VARCHAR); would strict set the value as VARCHAR.
https://www.postgresql.org/message-id/Pine.BSO.4.64.0702141439480.24142%40leary.csoft.net
But this seams not to happen in my case.
I have also testet to bind the value with:
stat.setNull(1, Types.NULL); or stat.setNull(1, Types.OTHER); or stat.setString(1, null);
Nothing works in this case.
As far as I understand, the jdbc driver sends the value without type information to the database. The database throws the exception.
So maybe this is a database problem.
The database can assume, that in case of ? is null it doesn't matter, which type the sent null is. So even a UNKOWN null is null or not null.
Strange is, that the following statement will work. So in this case null is unknown, but mapped.
conn.prepareStatement("select 1 where coalesce(?, null) is null");
This is indeed how all of this works in the current design. I suggest you add a cast to the input parameter in the query. Or choose a different value for stringtype…
David J.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: stringtype=unspecified is null check problem
@ 2023-01-11 13:34 Dave Cramer <[email protected]>
parent: Martin Handsteiner <[email protected]>
1 sibling, 0 replies; 14+ messages in thread
From: Dave Cramer @ 2023-01-11 13:34 UTC (permalink / raw)
To: Martin Handsteiner <[email protected]>; +Cc: David G. Johnston <[email protected]>; [email protected] <[email protected]>
Dave Cramer
www.postgres.rocks
On Wed, 11 Jan 2023 at 04:48, Martin Handsteiner <
[email protected]> wrote:
> There are 3 use cases, where I would need one setting, that always
> ensures, that null can be bound…
>
> (setNull(1, <setting>) and stringtype=<setting>)
>
>
>
> select 1 where 1=? -- setNull(1, Types.VARCHAR) and
> stringtype=unspecified
>
> select 1 where 'A'=? -- setNull(1, Types.VARCHAR) and stringtype
> doesn’t matter
>
> select 1 where ? is null -- setNull(1, Types.VARCHAR) and
> stringtype=VARCHAR
>
>
>
> That there is no way to binding null in a simple way is a bug for me,
> because the following will work, and the database has also to decide, how
> to map null:
>
>
>
> select 1 where 1=null -- now the database converts null to a number
>
> select 1 where 'A'=null -- now the database converts null to a varchar
>
> select 1 where null is null – now the database doesn’t care… so everthing
> works.
>
>
>
> The question is, what is the difference between the two, and why should
> the caller parse the statement, if he uses jdbc. These examples are easy,
> but in real world, in 1=? the 1 could also be a sub select (select max(id)
> from …)
>
>
>
One thing you should be aware of is that by default jdbc uses Extended
Query whereas psql uses simple query.
see PostgreSQL: Documentation: 15: 55.1. Overview
<https://www.postgresql.org/docs/current/protocol-overview.html#PROTOCOL-QUERY-CONCEPTS;
for
details on extended query.
Simple query allows the server to infer quite a bit whereas Extended Query
is very opinionated.
Dave
> *Von:* David G. Johnston <[email protected]>
> *Gesendet:* Dienstag, 10. Jänner 2023 18:44
> *An:* Martin Handsteiner <[email protected]>
> *Cc:* [email protected]
> *Betreff:* stringtype=unspecified is null check problem
>
>
>
> On Tuesday, January 10, 2023, Martin Handsteiner <
> [email protected]> wrote:
>
> If a bind parameter is checked for null, ERROR: could not determine data
> type of parameter $1 occurs.
> Simple testcase:
> Connection conn = DriverManager.getConnection("jdbc:postgresql://
> 192.168.1.201:5432/testdb?stringtype=unspecified", "test", "test");
> PreparedStatement stat = conn.prepareStatement("select 1 where ? is null");
> stat.setNull(1, Types.VARCHAR);
> stat.executeQuery();
> -> Exception
>
> For checking the parameter for null, the data type should not be relevant.
> At least TEXT or VARCHAR could be implicitly used.
>
> There is a mailing post, that says, that stat.setNull(1, Types.VARCHAR);
> would strict set the value as VARCHAR.
>
> https://www.postgresql.org/message-id/Pine.BSO.4.64.0702141439480.24142%40leary.csoft.net
> But this seams not to happen in my case.
>
> I have also testet to bind the value with:
> stat.setNull(1, Types.NULL); or stat.setNull(1, Types.OTHER); or
> stat.setString(1, null);
> Nothing works in this case.
>
> As far as I understand, the jdbc driver sends the value without type
> information to the database. The database throws the exception.
> So maybe this is a database problem.
> The database can assume, that in case of ? is null it doesn't matter,
> which type the sent null is. So even a UNKOWN null is null or not null.
>
> Strange is, that the following statement will work. So in this case null
> is unknown, but mapped.
> conn.prepareStatement("select 1 where coalesce(?, null) is null");
>
>
>
> This is indeed how all of this works in the current design. I suggest you
> add a cast to the input parameter in the query. Or choose a different
> value for stringtype…
>
>
>
> David J.
>
>
>
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: stringtype=unspecified is null check problem
@ 2023-01-12 00:48 David G. Johnston <[email protected]>
parent: Martin Handsteiner <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: David G. Johnston @ 2023-01-12 00:48 UTC (permalink / raw)
To: Martin Handsteiner <[email protected]>; +Cc: [email protected] <[email protected]>
On Wed, Jan 11, 2023 at 2:48 AM Martin Handsteiner <
[email protected]> wrote:
> There are 3 use cases, where I would need one setting, that always
> ensures, that null can be bound…
>
> (setNull(1, <setting>) and stringtype=<setting>)
>
>
>
> select 1 where 1=? -- setNull(1, Types.VARCHAR) and
> stringtype=unspecified
>
> select 1 where 'A'=? -- setNull(1, Types.VARCHAR) and stringtype
> doesn’t matter
>
> select 1 where ? is null -- setNull(1, Types.VARCHAR) and
> stringtype=VARCHAR
>
>
>
> That there is no way to binding null in a simple way is a bug for me,
> because the following will work, and the database has also to decide, how
> to map null:
>
>
>
> select 1 where 1=null -- now the database converts null to a number
>
> select 1 where 'A'=null -- now the database converts null to a varchar
>
> select 1 where null is null – now the database doesn’t care… so everthing
> works.
>
>
>
> The question is, what is the difference between the two, and why should
> the caller parse the statement, if he uses jdbc. These examples are easy,
> but in real world, in 1=? the 1 could also be a sub select (select max(id)
> from …)
>
>
The equality operator forces both sides of it to be of the same type.
Since the unquoted number 1 is a typed integer that fixes the null to be an
integer. Likewise, since both the single-quoted A and null both end up
being interpreted as text that is what you get. The "is null" test, unlike
the equals operator, does not force any particular concrete data type,
hence the error. You've asked the driver to not specify a concrete type
when sending text-like content and the server, respecting that, realizes it
cannot infer one either, and bam!
SELECT 1 where 1=?; and then using Types.VARCHAR is wrong on its face. The
fact is stringtype lets you write bad code and get away with it in some
limited cases. But as you've seen sometimes it doesn't and you have to
just go and write good code. Good, in this case, meaning being explicit
and accurate about the data types the query involves.
I'll agree this maybe isn't the best UX, but the few complainers are also
not stepping up and proposing a comprehensive fix. Call it a bug if it
makes you feel better but sometimes non-critical bugs don't get fixed; and
given the long history of this behavior, and complaints about it, I'm not
hopeful a change is going to happen.
I will say that a nearby thread on the server lists is discussing why null
defaults to be considered text; and to maybe change that. I suppose the
question here, for the server, is why it cannot just choose text instead of
producing "could not determine data type of parameter". Turning an error
into minimally functioning behavior seems like a reasonable consideration.
David J.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: stringtype=unspecified is null check problem
@ 2023-01-12 01:33 Tom Lane <[email protected]>
parent: David G. Johnston <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lane @ 2023-01-12 01:33 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: Martin Handsteiner <[email protected]>; [email protected] <[email protected]>
"David G. Johnston" <[email protected]> writes:
> The equality operator forces both sides of it to be of the same type.
> Since the unquoted number 1 is a typed integer that fixes the null to be an
> integer. Likewise, since both the single-quoted A and null both end up
> being interpreted as text that is what you get. The "is null" test, unlike
> the equals operator, does not force any particular concrete data type,
> hence the error. You've asked the driver to not specify a concrete type
> when sending text-like content and the server, respecting that, realizes it
> cannot infer one either, and bam!
Yeah. I concur that this is annoying, but the law of conservation of
cruft means that we can't easily fix it without creating new warts.
It's possible to experiment with this behavior without messing with
extended query mode, by seeing what PREPARE does with unspecified
parameters:
regression=# prepare foo as select $1 is null;
ERROR: could not determine data type of parameter $1
The complained-of problem. Annoying, especially since the seemingly
comparable
regression=# prepare foo as select $1 is true;
PREPARE
works fine. But that's not really comparable, since the IS TRUE
context offers the hint we need about the data type of $1.
You can fix it by providing an explicit statement of the intended
data type:
regression=# prepare foo as select $1::text is null;
PREPARE
and what the OP seems to wish is that the server would do that
automatically. Trouble is, what if the parameter is in fact
*not* text? That would get us into trouble with something like
regression=# prepare foo2 as select $1 is null or $1 = 42;
ERROR: operator does not exist: text = integer
LINE 1: prepare foo2 as select $1 is null or $1 = 42;
^
HINT: No operator matches the given name and argument types. You might need to add explicit type casts.
Maybe that's still better than the current behavior, in that
it's strictly fewer failures. But I'm worried about queries
silently succeeding with different semantics than the user
expected, which this seems like it'd open the door to.
(In theory maybe we could fix this with two passes over the
query, but I don't really want to go there.)
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: stringtype=unspecified is null check problem
@ 2023-01-12 01:59 David G. Johnston <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: David G. Johnston @ 2023-01-12 01:59 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Martin Handsteiner <[email protected]>; [email protected] <[email protected]>
On Wed, Jan 11, 2023 at 6:33 PM Tom Lane <[email protected]> wrote:
> "David G. Johnston" <[email protected]> writes:
> > The equality operator forces both sides of it to be of the same type.
> > Since the unquoted number 1 is a typed integer that fixes the null to be
> an
> > integer. Likewise, since both the single-quoted A and null both end up
> > being interpreted as text that is what you get. The "is null" test,
> unlike
> > the equals operator, does not force any particular concrete data type,
> > hence the error. You've asked the driver to not specify a concrete type
> > when sending text-like content and the server, respecting that, realizes
> it
> > cannot infer one either, and bam!
>
> Yeah. I concur that this is annoying, but the law of conservation of
> cruft means that we can't easily fix it without creating new warts.
>
> It's possible to experiment with this behavior without messing with
> extended query mode, by seeing what PREPARE does with unspecified
> parameters:
>
> regression=# prepare foo as select $1 is null;
> ERROR: could not determine data type of parameter $1
>
I'm just thinking that if at the end of the parse the system is left with
"unknown" as a parameter type it chooses text, just like seems to happen
most other places in the server during type resolution. Even if doing that
takes a second pass it seems such a pass would only ever have to occur when
this specific error arises. But I agree that if it takes a second pass it
is much less appealing to work out what those mechanics look like compared
to just a deferred reclassification if this specific error is detected.
regression=# prepare foo as select $1::text is null;
> PREPARE
>
> and what the OP seems to wish is that the server would do that
> automatically.
Right, just not explicitly like that or for every parameter, always., just
the ones that lack any other context fixing their type.
Trouble is, what if the parameter is in fact
> *not* text? That would get us into trouble with something like
>
> regression=# prepare foo2 as select $1 is null or $1 = 42;
> ERROR: operator does not exist: text = integer
> LINE 1: prepare foo2 as select $1 is null or $1 = 42;
> ^
> HINT: No operator matches the given name and argument types. You might
> need to add explicit type casts.
>
.
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.
We've just introduced soft-error handling for data type handling. Is there
any way to leverage some of that to turn the specific error into a soft
one, place references to problematic Vars somewhere, then reclassify them
at the end of the parse?
David J.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: stringtype=unspecified is null check problem
@ 2023-01-12 03:13 Tom Lane <[email protected]>
parent: David G. Johnston <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: Tom Lane @ 2023-01-12 03:13 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: Martin Handsteiner <[email protected]>; [email protected] <[email protected]>
"David G. Johnston" <[email protected]> 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.
regards, tom lane
Attachments:
[text/x-diff] resolve-unknown-params-better-0.1.patch (4.4K, ../../[email protected]/2-resolve-unknown-params-better-0.1.patch)
download | inline diff:
diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c
index 2240284f21..cbd70e3315 100644
--- a/src/backend/parser/parse_param.c
+++ b/src/backend/parser/parse_param.c
@@ -49,6 +49,7 @@ typedef struct VarParamState
{
Oid **paramTypes; /* array of parameter type OIDs */
int *numParams; /* number of array entries */
+ bool haveUnresolved; /* have we made any Params of type UNKNOWN? */
} VarParamState;
static Node *fixed_paramref_hook(ParseState *pstate, ParamRef *pref);
@@ -87,6 +88,7 @@ setup_parse_variable_parameters(ParseState *pstate,
parstate->paramTypes = paramTypes;
parstate->numParams = numParams;
+ parstate->haveUnresolved = false;
pstate->p_ref_hook_state = (void *) parstate;
pstate->p_paramref_hook = variable_paramref_hook;
pstate->p_coerce_param_hook = variable_coerce_param_hook;
@@ -168,6 +170,10 @@ variable_paramref_hook(ParseState *pstate, ParamRef *pref)
if (*pptype == VOIDOID && pstate->p_expr_kind == EXPR_KIND_CALL_ARGUMENT)
*pptype = UNKNOWNOID;
+ /* If we've not yet resolved the type, remember we need to fix it later */
+ if (*pptype == UNKNOWNOID)
+ parstate->haveUnresolved = true;
+
param = makeNode(Param);
param->paramkind = PARAM_EXTERN;
param->paramid = paramno;
@@ -257,30 +263,31 @@ variable_coerce_param_hook(ParseState *pstate, Param *param,
}
/*
- * Check for consistent assignment of variable parameters after completion
+ * Check for complete assignment of variable parameters after completion
* of parsing with parse_variable_parameters.
*
* Note: this code intentionally does not check that all parameter positions
- * were used, nor that all got non-UNKNOWN types assigned. Caller of parser
- * should enforce that if it's important.
+ * were used. Caller of parser should enforce that if it's important.
*/
void
check_variable_parameters(ParseState *pstate, Query *query)
{
VarParamState *parstate = (VarParamState *) pstate->p_ref_hook_state;
- /* If numParams is zero then no Params were generated, so no work */
- if (*parstate->numParams > 0)
+ /* Needn't do anything unless we made some UNKNOWN Params */
+ if (parstate->haveUnresolved)
(void) query_tree_walker(query,
check_parameter_resolution_walker,
(void *) pstate, 0);
}
/*
- * Traverse a fully-analyzed tree to verify that parameter symbols
- * match their types. We need this because some Params might still
- * be UNKNOWN, if there wasn't anything to force their coercion,
- * and yet other instances seen later might have gotten coerced.
+ * Traverse a fully-analyzed tree to back-fill any Params whose types were not
+ * determined at creation or during local parse analysis. This would only be
+ * possible in contexts where we did not have any need to force immediate
+ * resolution of the Param's type, for example "SELECT ... WHERE $1 IS NULL".
+ * If we have no other clue about the type of such a Param, resolve it as
+ * text, following the precedent of unknown-type literals.
*/
static bool
check_parameter_resolution_walker(Node *node, ParseState *pstate)
@@ -295,6 +302,7 @@ check_parameter_resolution_walker(Node *node, ParseState *pstate)
{
VarParamState *parstate = (VarParamState *) pstate->p_ref_hook_state;
int paramno = param->paramid;
+ Oid resolvedType;
if (paramno <= 0 || /* shouldn't happen, but... */
paramno > *parstate->numParams)
@@ -303,7 +311,26 @@ check_parameter_resolution_walker(Node *node, ParseState *pstate)
errmsg("there is no parameter $%d", paramno),
parser_errposition(pstate, param->location)));
- if (param->paramtype != (*parstate->paramTypes)[paramno - 1])
+ resolvedType = (*parstate->paramTypes)[paramno - 1];
+
+ /* If no use of the Param identified a type, resolve it as TEXT */
+ if (resolvedType == UNKNOWNOID)
+ {
+ resolvedType = TEXTOID;
+ (*parstate->paramTypes)[paramno - 1] = resolvedType;
+ }
+
+ /* If this particular Param wasn't resolved previously, fix it */
+ if (param->paramtype == UNKNOWNOID)
+ {
+ param->paramtype = resolvedType;
+ /* Update typmod and collation the same as above */
+ param->paramtypmod = -1;
+ param->paramcollid = get_typcollation(resolvedType);
+ }
+
+ /* Cross-check, just for paranoia */
+ else if (param->paramtype != resolvedType)
ereport(ERROR,
(errcode(ERRCODE_AMBIGUOUS_PARAMETER),
errmsg("could not determine data type of parameter $%d",
^ permalink raw reply [nested|flat] 14+ messages in thread
* AW: stringtype=unspecified is null check problem
@ 2023-01-12 08:32 Martin Handsteiner <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Martin Handsteiner @ 2023-01-12 08:32 UTC (permalink / raw)
To: Tom Lane <[email protected]>; David G. Johnston <[email protected]>; +Cc: [email protected] <[email protected]>
Thanks a lot, I think, this will help a lot of (java) users, and makes the usage of PostgreSQL DB a way better!
I had yesterday an idea for a solution on java side. As we have already a preprocessing with a jdbc wrapper, for function and procedure calls (in/out parameters are not working with postgres jdbc), I'll added preprocessing for the parameters.
I'll search the statement for all question marks (?) that are not inside a comment (-- \n, /* */), not in quotes (" ") and not in strings (' '), check the parameter, if it is null, and replace the ? by null, and reduce the parameter list.
I can link the code if you want, as it is Apache License 2.0.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: stringtype=unspecified is null check problem
@ 2023-01-12 14:36 Dave Cramer <[email protected]>
parent: Martin Handsteiner <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Dave Cramer @ 2023-01-12 14:36 UTC (permalink / raw)
To: Martin Handsteiner <[email protected]>; +Cc: Tom Lane <[email protected]>; David G. Johnston <[email protected]>; [email protected] <[email protected]>
On Thu, 12 Jan 2023 at 03:32, Martin Handsteiner <
[email protected]> wrote:
> Thanks a lot, I think, this will help a lot of (java) users, and makes the
> usage of PostgreSQL DB a way better!
>
> I had yesterday an idea for a solution on java side. As we have already a
> preprocessing with a jdbc wrapper, for function and procedure calls (in/out
> parameters are not working with postgres jdbc), I'll added preprocessing
> for the parameters.
>
> I'll search the statement for all question marks (?) that are not inside a
> comment (-- \n, /* */), not in quotes (" ") and not in strings (' '), check
> the parameter, if it is null, and replace the ? by null, and reduce the
> parameter list.
>
> I can link the code if you want, as it is Apache License 2.0.
>
Please do
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: stringtype=unspecified is null check problem
@ 2023-01-12 20:40 David G. Johnston <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: David G. Johnston @ 2023-01-12 20:40 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Martin Handsteiner <[email protected]>; [email protected] <[email protected]>
On Wed, Jan 11, 2023 at 8:13 PM Tom Lane <[email protected]> wrote:
> "David G. Johnston" <[email protected]> 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.
^ permalink raw reply [nested|flat] 14+ messages in thread
* AW: stringtype=unspecified is null check problem
@ 2023-01-13 09:54 Martin Handsteiner <[email protected]>
parent: Dave Cramer <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Martin Handsteiner @ 2023-01-13 09:54 UTC (permalink / raw)
To: Dave Cramer <[email protected]>; +Cc: Tom Lane <[email protected]>; David G. Johnston <[email protected]>; [email protected] <[email protected]>
Whole Source is too much, as there is a lot of other things inside:
Null Replacement went into general DBAccess, not only in special PostgreSQLDBAccess like executeFunction or executeProcedure.
https://sourceforge.net/p/jvx/code/HEAD/tree/trunk/java/library/src/com/sibvisions/rad/persist/jdbc/...
Relevant Source with example:
String originalStatement = "select 1 where ? is null";
Object[] originalParams = new Object[] {null};
StringBuilder statementToBeChanged = new StringBuilder(originalStatement);
Object[] newParams = replaceNullParameter(statementToBeChanged, originalParams);
String newStatement = statementToBeChanged.toString();
Real usage would be:
List<Bean> result = dba.executeQuery("select 1 where ? is null or 1 = ?", null, null);
/**
* Replaces null parameters (?) directly with null, to avoid cast exceptions.
*
* @param pStatement the original statement
* @param pParameters the parameters
* @return the new reduced parameters
*/
protected Object[] replaceNullParameter(StringBuilder pStatement, Object[] pParameters)
{
int[] paramIndexes = getParameterIndexes(pStatement.toString());
for (int i = paramIndexes.length - 1; i >= 0; i--)
{
Object param = pParameters[i];
if (AbstractParam.getValue(param) == null)
{
int index = paramIndexes[i];
if (index < pStatement.length() - 1 && !Character.isWhitespace(pStatement.charAt(index + 1)))
{
pStatement.insert(index + 1, " ");
}
pStatement.replace(index, index + 1, "null");
if (index > 0 && !Character.isWhitespace(pStatement.charAt(index - 1)))
{
pStatement.insert(index, " ");
}
pParameters = ArrayUtil.remove(pParameters, i);
}
}
return pParameters;
}
/**
* Gets the indexes of parameters (?).
*
* @param pStatement the statement
* @return the indexes of parameters (?)
*/
protected int[] getParameterIndexes(String pStatement)
{
int[] result = new int[0];
String sOpenQu = getOpenQuoteCharacter();
String sCloseQu = getCloseQuoteCharacter();
if (StringUtil.isEmpty(sOpenQu) || StringUtil.isEmpty(sCloseQu))
{
sOpenQu = null;
sCloseQu = null;
}
String sCloseMark = null;
for (int idx = 0, cnt = pStatement.length(); idx < cnt; idx++)
{
if (sCloseMark != null)
{
if (pStatement.startsWith(sCloseMark, idx))
{
idx += sCloseMark.length() - 1;
sCloseMark = null;
}
}
else
{
if (pStatement.startsWith("\'", idx))
{
sCloseMark = "\'";
}
else if (pStatement.startsWith("\"", idx))
{
sCloseMark = "\"";
}
else if (sOpenQu != null && pStatement.startsWith(sOpenQu, idx))
{
idx += sOpenQu.length() - 1;
sCloseMark = sCloseQu;
}
else if (pStatement.startsWith("--", idx))
{
idx++;
sCloseMark = "\n";
}
else if (pStatement.startsWith("/*", idx))
{
idx++;
sCloseMark = "*/";
}
else if (pStatement.startsWith("?", idx))
{
result = ArrayUtil.add(result, idx);
}
}
}
return result;
}
^ permalink raw reply [nested|flat] 14+ messages in thread
* AW: stringtype=unspecified is null check problem
@ 2023-01-13 10:16 Martin Handsteiner <[email protected]>
parent: David G. Johnston <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Martin Handsteiner @ 2023-01-13 10:16 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; Tom Lane <[email protected]>; +Cc: [email protected] <[email protected]>
> 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');
I know, this test case is not about problems with calling functions with jdbc, but as we are still in jdbc mailing list:
A good test case for java would be a function with mixed in / out params, first in then out.
create or replace function callfunc(in val1 integer, in val2 integer, in out val3 integer, out val4 integer) returns void
The out values are mapped to the wrong indexes 1 and 2 because result from db is {1, 1} but jdbc throws exception, that 1 and 2 is not registered as out param…
So calling functions with out param from jdbc only works when out params are first.
We solved that by calling functions with
SELECT * FROM callfunc(?, ?, ?)
And mapping the result by our own to the correct indexes.
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2023-01-13 10:16 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-01-10 17:28 stringtype=unspecified is null check problem Martin Handsteiner <[email protected]>
2023-01-10 17:44 ` David G. Johnston <[email protected]>
2023-01-10 18:53 ` Martin Handsteiner <[email protected]>
2023-01-11 09:48 ` AW: stringtype=unspecified is null check problem Martin Handsteiner <[email protected]>
2023-01-11 13:34 ` Dave Cramer <[email protected]>
2023-01-12 00:48 ` David G. Johnston <[email protected]>
2023-01-12 01:33 ` Tom Lane <[email protected]>
2023-01-12 01:59 ` David G. Johnston <[email protected]>
2023-01-12 03:13 ` Tom Lane <[email protected]>
2023-01-12 08:32 ` AW: stringtype=unspecified is null check problem Martin Handsteiner <[email protected]>
2023-01-12 14:36 ` Dave Cramer <[email protected]>
2023-01-13 09:54 ` AW: stringtype=unspecified is null check problem Martin Handsteiner <[email protected]>
2023-01-12 20:40 ` David G. Johnston <[email protected]>
2023-01-13 10:16 ` AW: stringtype=unspecified is null check problem Martin Handsteiner <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox