Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare confuses types when more than one argument is used #6724

Closed
monetdb-team opened this issue Nov 30, 2020 · 0 comments
Closed

Prepare confuses types when more than one argument is used #6724

monetdb-team opened this issue Nov 30, 2020 · 0 comments

Comments

@monetdb-team
Copy link

@monetdb-team monetdb-team commented Nov 30, 2020

Date: 2019-07-04 17:37:14 +0200
From: @swingbit
To: SQL devs <>
Version: 11.31.13 (Aug2018-SP2)
CC: daniel.zvinca, @PedroTadim

Last updated: 2019-12-20 15:36:33 +0100

Comment 27107

Date: 2019-07-04 17:37:14 +0200
From: @swingbit

User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.100 Safari/537.36
Build Identifier:

start transaction;

create function mylength1(s string) returns int
begin
return length(s);
end;

create function mylength2(s string, i int) returns int
begin
return length(s) + i;
end;

  • This is correct:

sql>prepare select mylength1(?);
execute prepared statement using: EXEC 9(...)
+------+--------+-------+--------+-------+--------+
| type | digits | scale | schema | table | column |
+======+========+=======+========+=======+========+
| int | 32 | 0 | | L2 | L2 |
| clob | 0 | 0 | null | null | null |
+------+--------+-------+--------+-------+--------+
2 tuples

  • This is wrong (the string argument becomes hugeint):

sql>prepare select mylength2(?, 2);
execute prepared statement using: EXEC 11(...)
+---------+--------+-------+--------+-------+--------+
| type | digits | scale | schema | table | column |
+=========+========+=======+========+=======+========+
| int | 32 | 0 | | L2 | L2 |
| hugeint | 128 | 0 | null | null | null |
+---------+--------+-------+--------+-------+--------+
2 tuples

Reproducible: Always

Comment 27109

Date: 2019-07-05 08:04:24 +0200
From: daniel.zvinca

Obviously it is an issue here. But the fix should consider also the tricky case of overloaded functions (same name, different parameter types, return type included). The followings are valid:

create function my_func(s string, i int) returns int
begin
return length(s) + i;
end;

create function my_func(s bigint, i int) returns int
begin
return i;
end;

create function my_func(s string, i int) returns string
begin
return coalesce(s, '');
end;

How the prepare/compiler would know what function to map to for “prepare select my_func(?, 2);”?

I can only guess that the following can work to some extents:

“prepare select cast(my_func(cast(? as string), 2) as int);”

But, it might be also an idea to allow the type indication “? as type” or “?:type” or "function{ret_type, param1_type, param2_type}. Something like.

"prepare select my_func(? as string, 2) as int;"

"prepare select my_func{string, string, i}(?, 2);"

Anyway for the given example of non-overloaded functions I expect the param type to be known and for overloaded functions throwing an error would be the safest way (in case the overloaded functions have incompatible types for the variable parameter or always ?).

Comment 27111

Date: 2019-07-05 11:57:00 +0200
From: @swingbit

Daniel,

The overloaded signatures are indeed yet another problem.
At the moment we solve that issue in most cases using an explicit cast, as in

prepare select my_func(cast(? as string), 2);

Explicit syntax could also be nice.

Perhaps this would be the right occasion to extend the syntax with allowing named prepared statement. This is something that most SQL vendors have and would be a very nice addition.

For example, PostgreSQL's syntax solves both issues:

prepare my_prep(string, int) as select my_func(?,2);

exec my_prep('hello');

I would find this an elegant solution (the current syntax could even be kept for backward compatibility)

Comment 27112

Date: 2019-07-05 13:37:37 +0200
From: daniel.zvinca

PostgreSQL approach is elegant, indeed. I don't mind much the unique number returned by MonetDB, I can certainly live with that, but types should be possible to be mentioned somehow for the parameters. Cast is not exactly an indication for the expected type, but it seems the accepted solution for overloaded functions like built-in coalesce.

("Bonus", the server just crashed. It can be reproduced on Win64/Apr2019.
See further below).

sql> prepare select coalesce(?, 23);
result type missing

sql> prepare select coalesce(cast(? as int), 23);
execute prepared statement using: EXEC 5(...)

+------+--------+-------+--------+-------+--------+
| type | digits | scale | schema | table | column |
+======+========+=======+========+=======+========+
| int | 32 | 0 | | L3 | L3 |
| int | 32 | 0 | null | null | null |
+------+--------+-------+--------+-------+--------+

However, this is also how I was able to crash the server.

sql> exec 5();
(server crashed)
Connection terminated during read line
Press any key to continue . . .

The server seems to crash for any case where provided number of parameters is different than compiled one.

Eventually, the following that also uses an overloaded function, provides a guess I am not happy with (cast seems to be the only way to avoid the issue):

sql>prepare select abs(?);
execute prepared statement using: EXEC 7(...)
+--------------+--------+-------+--------+-------+--------+
| type | digits | scale | schema | table | column |
+==============+========+=======+========+=======+========+
| sec_interval | 13 | 0 | | L2 | L2 |
| sec_interval | 13 | 0 | null | null | null |
+--------------+--------+-------+--------+-------+--------+
2 tuples

sql>exec 7(12.5);
wrong type for argument 1 of function call: decimal, expected sec_interval

Comment 27114

Date: 2019-07-08 11:16:29 +0200
From: @PedroTadim

Hello Daniel,

Recently I also found the problem of calling exec with the wrong number of arguments. This issue should be already fixed.

Comment 27115

Date: 2019-07-08 12:03:05 +0200
From: MonetDB Mercurial Repository <>

Changeset cbd28b7ab3cb made by Pedro Ferreira pedro.ferreira@monetdbsolutions.com in the MonetDB repo, refers to this bug.

For complete details, see https//devmonetdborg/hg/MonetDB?cmd=changeset;node=cbd28b7ab3cb

Changeset description:

Added test for bug #6724.

Comment 27434

Date: 2019-11-30 10:28:27 +0100
From: MonetDB Mercurial Repository <>

Changeset c331fce0695b made by Niels Nes niels@cwi.nl in the MonetDB repo, refers to this bug.

For complete details, see https//devmonetdborg/hg/MonetDB?cmd=changeset;node=c331fce0695b

Changeset description:

add fix for prepare-types bug #6724
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant