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
Invalid data type for negation (minus operator) [CORE5395] #5668
Comments
Commented by: @sim1984 This regression is reproduced the same way and using the execute statement execute block |
Commented by: @asfernandes Saying 2.5 is right is half words. Would you consider this right? SQL> select 1 from TABLE1 where id = 'a' || ?; INPUT SQLDA version: 1 sqln: 10 sqld: 1 Of course it's not. It's very weird the way it infers a parameter datatype. |
Commented by: @hvlad Adriano, SQL> execute block returns (i int)
============
============ do you consider it correct ? |
Commented by: verleon (verleon) Hi! |
Commented by: Sean Leyne (seanleyne) Verleon, While it is an issue, there is a workaround... select 1 from TABLE1 where id = (:param) * -1 As the other examples show, there is no simple fix for your issue. So, you may need to help yourself by using workarounds. |
Modified by: @asfernandesassignee: Adriano dos Santos Fernandes [ asfernandes ] |
Commented by: verleon (verleon) Sean Leyne, In version 3.0.2., database in dialect 1, this query: select 1 from TABLE1 where id = (:param) * -1 also results a bug: Unsuccessful execution caused by a system error that precludes successful execution of subsequent statements. |
Commented by: Evgeniy Shubenkov (e.shubenkov) We have a similar problem that makes it very difficult to switch to Firebird 3.0.4 due to the large number of similar requests. An example of a working query (Firebird 2.5.7): CREATE TABLE PERERASHETCASE ( select LSHET from PERERASHETCASE result error on Firebird 3.0.4: Unsuccessful execution caused by a system error that precludes successful execution of subsequent statements. But adding "cast (: BEGYEAR as integer)" for only ONE parameter solves the problem: select LSHET from PERERASHETCASE |
Commented by: @hvlad Adriano, what do you think about following attempt to fix (regression, at least) diff --git a/src/dsql/pass1.cpp b/src/dsql/pass1.cpp - MAKE_desc(dsqlScratch, &node->nodDesc, node); v2.5 have no MAKE_desc() call in this place, AFAIU See quick comparison below: a) with no fix SQL> set sqlda_display on; INPUT message field count: 1 OUTPUT message field count: 1 b) with a fix SQL> set sqlda_display on; INPUT message field count: 3 OUTPUT message field count: 1 INPUT message field count: 1 OUTPUT message field count: 1 INPUT message field count: 1 OUTPUT message field count: 1 INPUT message field count: 1 OUTPUT message field count: 1 I.e. it fixed all issues with arithmetic operations and not changed data type for concatenation |
Commented by: @dyemanov > v2.5 have no MAKE_desc() call in this place, AFAIU I had the same idea, but v2.5 has MAKE_desc() inside PASS1_set_parameter_type() for nod_parameter, so IMO removing it completely is not absolutely correct. As it's hard to pass the node through the setParameterType() call stack (now it passes only the descriptor), I tried a different approach, see the attached patch. It also fixes all known regressions, although looks somewhat overcomplicated if compared to yours ;-) Maybe Adriano could say whether it's suitable or not, in his opinion. |
Modified by: @dyemanovAttachment: core-5395.diff [ 13310 ] |
Commented by: @hvlad > > v2.5 have no MAKE_desc() call in this place, AFAIU > I had the same idea, but v2.5 has MAKE_desc() inside PASS1_set_parameter_type() for nod_parameter, so IMO removing it completely is not absolutely correct. Do you speak about this code v2.5
--------- v3
--------- If yes, i think it is more or less the same. Note, "desc" in ParameterNode::setParameterType is the node->nodDesc passed at bool PASS1_set_parameter_type(DsqlCompilerScratch* dsqlScratch, ValueExprNode* inNode,
} and your patch just assign the same value to it. Am I wrong ? |
Commented by: @dyemanov Yep, I speak about that piece of code. MAKE_desc() should be called *only* for the node we're deriving the parameter datatype from. In v3, it's called for all nodes (both parts of comparison), including the part we haven't guessed yet, hence the errors reported in this ticket. v2.5 was passing the node through the call stack to make deferred MAKE_desc() possible. v3 passes only the descriptor, so it has to call MAKE_desc() before that (otherwise node->nodDesc has unknown datatype) and we have the issue. Deferred evaluation of this descriptor is impossible without the node which is not accessible when ParameterNode is processed. My patch stacks the current node until the parameter is processed, thus deferring MAKE_desc() call to its original location. In other words, if no parameters are found in one part of comparison (left or right), MAKE_desc() is not called for the opposite part (unlike what happens now). It's a simpler alternative to restoring the original logic which would mean to duplicate the setParameterType() method through all the ExprNode classes. I don't insist it's 100% correct, it was a just quick attempt to fix, now subject for discussion. |
Commented by: @hvlad Ok, i think we need Adriano's opinion to go forward |
Commented by: @asfernandes I'd go with Dmitry's solution. I think a question remains, if we need to adjust the "if (!desc)" to also check for "!dsqlScratch->paramTypeNodes.hasData()", as it seems it's what the original code does. |
Commented by: @dyemanov Fix is committed into v3.0.5, please test the next (tomorrow's) snapshot build and report back. Patch for v4 will follow shortly. |
Modified by: @dyemanovFix Version: 3.0.5 [ 10885 ] |
Commented by: Evgeniy Shubenkov (e.shubenkov) Tested successfully on Firebird-3.0.5.33083-0_x64. |
Modified by: @asfernandesstatus: Open [ 1 ] => Resolved [ 5 ] resolution: Fixed [ 1 ] Fix Version: 4.0 Beta 1 [ 10750 ] |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Resolved [ 5 ] QA Status: No test => Done successfully |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Closed [ 6 ] |
Submitted by: verleon (verleon)
Attachments:
core-5395.diff
Votes: 7
meta:
CREATE TABLE TABLE1 (
ID INTEGER
);
query:
select 1 from TABLE1 where id = - :param
result:
Unsuccessful execution caused by a system error that precludes successful execution of subsequent statements.
Dynamic SQL Error.
expression evaluation not supported.
Invalid data type for negation (minus operator).
This problem is gone when using - (cast :param as integer) instead of - :param, but with FB 2.5 it was not necessary
Commits: 4ce50c0 93da6eb
The text was updated successfully, but these errors were encountered: