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
Regression: v4 Beta 1 rejects POSITION element of v2.5 defined SQL2003 CREATE TRIGGER syntax [CORE6243] #6487
Comments
Commented by: Tony Whyman (twhyman) By "dumped" I did of course mean use isql -A to extract the DDL |
Commented by: @pavel-zotov May be this is the same as i've commented in CORE5545 22/Jun/17 05:11 AM ? |
Commented by: Tony Whyman (twhyman) It does look similar. The engine should be compliant with It all works fine in Firebird 3 so a regression has slipped in somewhere. |
Modified by: Sean Leyne (seanleyne)description: In a simple test of the Firebird 4 engine, I dumped the example employee database (from a Firebird 3 source) and edited the first CREATE TRIGGER to: CREATE TRIGGER SET_CUST_NO so that it was SQL 2003 compliant. When feeding the SQL back in to create a new database using the Firebird 4 isql, this caused the following error message: Statement failed, SQLSTATE = 42000 I also fed exactly the same script back to a Firebird 3 Server/isql and the script completed with no errors. => In a simple test of the Firebird 4 engine, I dumped the example employee database (from a Firebird 3 source) and edited the first CREATE TRIGGER to: CREATE TRIGGER SET_CUST_NO When feeding the SQL back in to create a new database using the Firebird 4 isql, this caused the following error message: Statement failed, SQLSTATE = 42000 I also fed exactly the same script back to a Firebird 3 Server/isql and the script completed with no errors. summary: Firebird 4 Beta 1 rejects SQL 2003 compliant CREATE TRIGGER syntax => Regression: v4 Beta 1 rejects valid CREATE TRIGGER syntax |
Commented by: @asfernandes The syntax is invalid accordingly to SQL 2013: <trigger definition> ::= BEFORE INSERT ON are fixed parts of a same clause. You should not inject different clauses (POSITION) between them. |
Commented by: Tony Whyman (twhyman) It is not unreasonable that Firebird should support SQL 2013 syntax. However, I can't find anything in the release notes with respect to SQL2013 compliance nor is this incompatibility with Firebird 3 mentioned in Chapter 12 (compatibility issues). At the very least, the Release Notes need updating. Is this the only change for SQL2013, or are there any others? Also, is there any reason to withdraw a feature present in both Firebird 2.5 and Firebird 3 and which will cause a compatibility problem for any users that adopted the SQL2003 syntax? On initial inspection, I can't see a good reason why the parser should not be able to cope with both SQL2003 and SQL2013 variants. |
Commented by: @asfernandes Show us where the SQL 2003 syntax has the POSITION clause. |
Commented by: Tony Whyman (twhyman) Copied from the Firebird 2.5 Language Reference (https://firebirdsql.org/file/documentation/reference_manuals/fblangref25-en/html/fblangref25-ddl-trgr.html) CREATE TRIGGER trigname { <relation_trigger_legacy> ::= <relation_trigger_sql2003> ::= <database_trigger> ::= <mutation_list> ::= <mutation> ::= { INSERT | UPDATE | DELETE } <db_event> ::= { <declarations> ::= {<declare_var> | <declare_cursor>}; |
Commented by: Sean Leyne (seanleyne) Adriano, Interbase/Firebird has always supported POSITION as part of Trigger definition (back to 1998), are you saying that with v4 this will no longer be supported? |
Modified by: Sean Leyne (seanleyne)summary: Regression: v4 Beta 1 rejects valid CREATE TRIGGER syntax => Regression: v4 Beta 1 rejects POSITION element in CREATE TRIGGER syntax |
Modified by: Sean Leyne (seanleyne)summary: Regression: v4 Beta 1 rejects POSITION element in CREATE TRIGGER syntax => Regression: v4 Beta 1 rejects POSITION element of v2.5 defined SQL2003 CREATE TRIGGER syntax |
Commented by: @asfernandes > Interbase/Firebird has always supported POSITION as part of Trigger definition (back to 1998), are you saying that with v4 this will no longer be supported? Of course it is not the case. POSITION is an extension, and it should not be between the fixed standard parts BEFORE INSERT ON <table>, in the same way it's not allowed INSERT BEFORE ON <table>, INSERT ON <table> BEFORE or another bizarre variant. |
Commented by: Tony Whyman (twhyman) So how is "POSITION" supported in the SQL2013 variant? |
Commented by: @asfernandes > So how is "POSITION" supported in the SQL2013 variant? As the last subclause. |
Commented by: Sean Leyne (seanleyne) So, the language reference should read as follows (correct?): CREATE TRIGGER trigname { <relation_trigger_legacy> ::= <relation_trigger_sql2003> ::= <database_trigger> ::= ... |
Commented by: @reevespaul It doesn't really matter if the old syntax was an extension, or legacy, or whatever we want to call it. It has been part of the trigger syntax of firebird since InterBase days. Conformance to a level of the SQL standard is an ideal, not an obligation. I don' t think they hand out prizes for conformance, that's for sure. But I do know that breaking code like this doesn't win any friends, especially when the only justification is conformance to an arbitrary standard. |
Commented by: Tony Whyman (twhyman) Running a few tests: Test 1CREATE TRIGGER SET_CUST_NO works for Firebird 4 Beta 1, but fails with Firebird 3.0.5. Test 2CREATE TRIGGER SET_CUST_NO works for Firebird 3.0.5, but fails with Firebird 4 Beta 1 Test 3CREATE TRIGGER SET_CUST_NO FOR CUSTOMER works for both Firebird 3.0.5 and Firebird 4 Beta 1. I found this issue when testing the IBX TIBExtract component with Firebird 4 Beta 1. Several iterations ago and when adding support for database triggers, I had changed this to use the "SQL2003 syntax". If nothing changes then it looks like I will have to revert back to using the legacy syntax, as that is the only one that works reliably over Firebird 2.5, 3 and 4. (isql also uses the legacy syntax when it too extracts the database schema). I agree with Paul Reeve's comment. Your users never forgive you for breaking existing code unless there is a very good reason - and coding elegance is never one of them. By all means add support for the "correct syntax" i.e. "ON CUSTOMER POSITION 0" and encourage its use - but Firebird 4 should still accept the old "POSITION 0 ON CUSTOMER" style as well. Whatever the outcome, please also make sure that the Release Notes reflect the changes to the CREATE TRIGGER syntax. |
Commented by: @asfernandes > Conformance to a level of the SQL standard is an ideal, not an obligation. I don' t think they hand out prizes for conformance, that's for sure. But I do know that breaking code like this doesn't win any friends, especially when the only justification is conformance to an arbitrary standard. I do not know where you read that! I can revisit details of CORE5545 (and probably emails about the subject) because the people so interested now didn't payed attention then! Of course it will take much more time than if the interested people payed attention 3 years ago. |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Resolved [ 5 ] QA Status: No test => Covered by another test(s) Test Details: See test for CORE5545. |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Closed [ 6 ] |
Commented by: Tony Whyman (twhyman) > I can revisit details of CORE5545 (and probably emails about the subject) because the people so interested now didn't payed attention then! >Of course it will take much more time than if the interested people payed attention 3 years ago. Except that CORE5545 did include the request "please ensure this is fixed in a backwards compatible way". |
Modified by: @asfernandesstatus: Closed [ 6 ] => Reopened [ 4 ] assignee: Adriano dos Santos Fernandes [ asfernandes ] resolution: Won't Fix [ 2 ] => |
Modified by: @asfernandesstatus: Reopened [ 4 ] => Resolved [ 5 ] resolution: Fixed [ 1 ] Fix Version: 4.0 Beta 2 [ 10888 ] |
Submitted by: Tony Whyman (twhyman)
In a simple test of the Firebird 4 engine, I dumped the example employee database (from a Firebird 3 source) and edited the first CREATE TRIGGER to:
CREATE TRIGGER SET_CUST_NO
ACTIVE BEFORE INSERT POSITION 0
ON CUSTOMER
AS
BEGIN
if (new.cust_no is null) then
new.cust_no = gen_id(cust_no_gen, 1);
END ^
When feeding the SQL back in to create a new database using the Firebird 4 isql, this caused the following error message:
Statement failed, SQLSTATE = 42000
Dynamic SQL Error
-SQL error code = -104
-Token unknown - line 2, column 22
-POSITION
I also fed exactly the same script back to a Firebird 3 Server/isql and the script completed with no errors.
Commits: d99fcf9
====== Test Details ======
See test for CORE5545.
The text was updated successfully, but these errors were encountered: