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

Execute statement with excess parameters [CORE5658] #5924

Closed
firebird-issue-importer opened this issue Nov 11, 2017 · 27 comments
Closed

Execute statement with excess parameters [CORE5658] #5924

firebird-issue-importer opened this issue Nov 11, 2017 · 27 comments

Comments

@firebird-issue-importer

Submitted by: Vladimir Arkhipov (arkinform)

Is duplicated by CORE4736

Votes: 1

I want to suggest "optional" directive in execute statement with named parameters:

EXECUTE STATEMENT (SQL_TEXT) (PARAM1 := VALUE1, OPTIONAL PARAM2 := VALUE2)

If PARAM2 is not found by name then simply don't set optional PARAM2 value in statement instead of "input parameters mismatch" error.

We have very complex stored procedures, where some parts of sql (joins, conditions, derived tables and etc) can be added dynamically depending on input procedure parameters.

For example (the real procedures are more complex):

create or alter procedure TEST_CLIENTS_LIST
(
search_name varchar(255),
search_phone varchar(64) = ''
)
returns
(
pcode bigint,
fullname varchar(255)
)
as
declare sql_text varchar(1024);
begin
sql_text = 'select c.pcode, c.fullname from clients c';

-- дополнительная фильтрация по номеру телефона
if (search_phone > '') then
sql_text = sql_text || ' left join clphones p on c.pcode = p.pcode';

-- обязательный поиск по имени
sql_text = sql_text || ' where c.fullname starting with :search_name';

-- дополнительная фильтрация по номеру телефона
if (search_phone > '') then
sql_text = sql_text || ' and p.phone = :search_phone';

for execute statement (sql_text)
(search_name := search_name,
search_phone := search_phone)
into pcode, fullname do
suspend;
end

After selecting from this procedure with "search_name" parameter without "search_phone" we get error "input parameters mismatch" because there is no "search_phone" parameter in sql text.
Unfortunately at present in these cases we often have to set parameter value directly in sql text without using parameters or make many "if" sections for executing sql with different parameters.
Although we could use "optional" syntax in these cases.

Commits: e17bff1 ee3a13d

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 12, 2017

Commented by: Omacht András (aomacht)

Same as CORE4736, but I voted for it! ;)

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 6, 2018

Commented by: @hvlad

Word OPTIONAL used with parameter suggest that caller could not set value for such parameter.
You need different behaviour - ability to silently ignore "marked" parameter if it is not present at SQL query text.
I suggest to "mark" such parameters using keyword EXTRA or EXCESS or ADDITIONAL.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 7, 2018

Commented by: Vladimir Arkhipov (arkinform)

I do not agree, I think that for case "caller could not set value for such parameter" word "NULLABLE" is more suitable.
Word "OPTIONAL" is more obvious compared to other suggested variants.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 7, 2018

Commented by: @asfernandes

I think this feature should not be done in this way.

It would be much better to think on terms of PSQL APis, as an example, as Oracle does with DBMS_SQL, where developer could dinamically inspect metadata and set parameters.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 7, 2018

Commented by: Vladimir Arkhipov (arkinform)

I wrote similar suggestion 8 years ago in CORE2813, but I think that OPTIONAL keyword is simple and very useful feature for simple use cases.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 7, 2018

Commented by: @hvlad

Vladimir,

There are some programming language which allows optional parameters. All of them (at least all that i know) requires default value for such parameter.
I.e. optional parameters is well know idea. It is also known as parameters with default values (or "default parameters").

You ask for completely different feature. In your case caller always set all declared parameters of EXECUTE STATEMENT and you want to allow to the
SQL query (i.e. "body" of EXECUTE STATEMENT) to not use some of declared params. From the SQL query point of view some input parameters are
not necessary to use. For me, word "extra" or "excess" looks very logical as mark for such parameters. But not "optional" as it is already widely used
with different meaning.

I don't want to mix up well known "optinal\default parameters" feature with feature you asked for.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 7, 2018

Commented by: @hvlad

Adriano,

with all respect to Oracle, i don't think we should duplicate ISC API at P-SQL level (or copy DBMS_SQL package).
This way is very error prone, looks ugly (from SQL POV) and (i believe) will not be widely used by the most FB users.

I will not resist if such package arrives at Firebird some day, but i definitely will not be happy with it ;)

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 7, 2018

Commented by: @asfernandes

Vladimir, it's already possible to pass parameters not used. Just use RDB$SET_CONTEXT / RDB$GET_CONTEXT.

Not any static language (as PSQL) allows caller to pass unrecognized parameters.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 7, 2018

Commented by: Vladimir Arkhipov (arkinform)

Adriano, I don't want to "pass unrecognized parameters", I want to "don't pass parameters" if there are no such parameters in sql text. At present I can make dynamic sql text but I can not make dynamic set of parameters.

Vlad, if you don't want to use OPTIONAL keyword, I choose ADDITIONAL.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 7, 2018

Commented by: @asfernandes

You want to pass unrecognized parameters, use context for it.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Feb 12, 2018

Commented by: Omacht András (aomacht)

Same as / related to CORE4736

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 18, 2018

Commented by: @hvlad

Adriano,

context variables can't help in case of external query

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 18, 2018

Commented by: @hvlad

Lets continue discussion.

I can imagine 3 ways to mark excess (extra, addtional) parameters

1. mark every parameter individually:

EXECUTE STATEMENT ('sql text') (in_param1, EXCESS in_param2, in_param3, EXCESS in_param4)

here marked in_param2 and in_param4

2. mark once all parameters at the tail of the parameters list:

EXECUTE STATEMENT ('sql text') (in_param1, in_param2, EXCESS in_param_3, in_param4)

here marked in_param3 and in_param4

3. put excess parameters into separate list:

EXECUTE STATEMENT ('sql text') (in_param1, in_param2) EXCESS (in_param_3, in_param4)

here marked in_param3 and in_param4

The syntax (1) is in-line with other languages where input parameters could be marked (IN, OUT, INOUT, REF etc)
The syntax (2) and (3) are less noisy. The (3) is very unusual...

Another question if we should allow to mark not named parameters. It introduces some kind of ambiguity, see

EXECUTE STATEMENT ('insert into x values(?, ?, ?)') (1, excess 2, excess 3, 4)

what should be inserted: (1, 2, 4) or (1, 3, 4) ?

With another syntax we also have problem

EXECUTE STATEMENT ('insert into x values(?, ?, ?)') (1, 4) EXCESS (2, 3)

how to put non-excess value into 3rd parameter and leave excess value (which one ?) for the 2nd one ?

With named params we have no such problem:

EXECUTE STATEMENT ('insert into x values(:a, :b, :c)') (a := 1, EXCESS b := 2, EXCESS x := 3, c := 4), or
EXECUTE STATEMENT ('insert into x values(:a, :b, :c)') (a := 1, c := 4, EXCESS b := 2, x := 3), or
EXECUTE STATEMENT ('insert into x values(:a, :b, :c)') (a := 1, c := 4) EXCESS (b := 2, x := 3)

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 19, 2018

Commented by: Vladimir Arkhipov (arkinform)

>> I can imagine 3 ways to mark excess (extra, addtional) parameters

I think that syntax #⁠1 is the best variant and the most evident.

>> Another question if we should allow to mark not named parameters. It introduces some kind of ambiguity, see

In this case must be error like "parameter mismatch"

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 20, 2018

Commented by: Omacht András (aomacht)

Hi Vlad!
I vote for #⁠1 too, but I suggest the 'optional' word instead of 'excess'.
András

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 20, 2018

Commented by: @hvlad

András,

thanks for opinion.
I already explained why OPTIONAL can't be used here. So - no - it will not be OPTIONAL.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 20, 2018

Commented by: Omacht András (aomacht)

Sorry Vlad, i forgot to reread comments. I can accept excess too.

András

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 4, 2019

Modified by: @hvlad

assignee: Vlad Khorsun [ hvlad ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 6, 2019

Commented by: @hvlad

Changed ticket title to reflect agreement in keyword used (optional -> excess)

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 6, 2019

Modified by: @hvlad

summary: Execute statement with optional parameters => Execute statement with excess parameters

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 6, 2019

Commented by: @hvlad

The patch is committed, please check next snapshot build

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 19, 2019

Modified by: @hvlad

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: 4.0 Beta 2 [ 10888 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 19, 2019

Commented by: Omacht András (aomacht)

Hi Vlad!

We tested it and works well.

András

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 2, 2019

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: No test => Done successfully

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 2, 2019

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Closed [ 6 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 2, 2019

Modified by: @hvlad

Link: This issue is duplicated by CORE4736 [ CORE4736 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 7, 2020

Commented by: @SENikitin

Can you please backport it to 3.0 too? This is very important for our business logic implementation.

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
2 participants