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

SELECT statements are processed for getGeneratedKeys by appending RETURNING (+ all columnnames) [JDBC391] #433

Closed
firebird-automations opened this issue Apr 20, 2015 · 10 comments

Comments

@firebird-automations
Copy link

Submitted by: @mrotteveel

The generated keys implementation assumes that the parser throws an exception on SELECT queries, and would then proceed executing the query normally. Unfortunately the parser doesn't throw an exception on a SELECT, and instead it processes the query by appending `RETURNING` + all columns in the database (which seems to be another bug in and of itself).

There are currently no tests that check the assumption that SELECT is treated unmodified.

See http://stackoverflow.com/questions/29752184/query-error-using-firebird-rdbms-with-coldfusion-11

Commits: ab19e0c 4ccd923 845ea7b 047825d 91e0013 9573cae d082495 15c8aeb FirebirdSQL/fbt-repository@fff75aa FirebirdSQL/fbt-repository@74b8e4f

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

Fix Version: Jaybird 2.2.8 [ 10664 ]

Fix Version: Jaybird 3.0 [ 10440 ]

description: The generated keys implementation assumes that the parser throws an exception on SELECT queries, and would then proceed normally. Unfortunately the parser doesn't throw an exception on a SELECT, and instead it processes the query by appending `RETURNING` + all columns in the database (which seems to be another bug in and of itself).

There are no tests that check the assumption that SELECT is treated unmodified.

See http://stackoverflow.com/questions/29752184/query-error-using-firebird-rdbms-with-coldfusion-11

=>

The generated keys implementation assumes that the parser throws an exception on SELECT queries, and would then proceed executing the query normally. Unfortunately the parser doesn't throw an exception on a SELECT, and instead it processes the query by appending `RETURNING` + all columns in the database (which seems to be another bug in and of itself).

There are currently no tests that check the assumption that SELECT is treated unmodified.

See http://stackoverflow.com/questions/29752184/query-error-using-firebird-rdbms-with-coldfusion-11

@firebird-automations
Copy link
Author

Commented by: @mrotteveel

Added initial test that demonstrates this is broken.

@firebird-automations
Copy link
Author

Commented by: @mrotteveel

Errors during parsing are now checked, and if there have been parser errors, or if no table name was found, then the query is executed as if it isn't a generated keys query. This fixes this problem (and some related issues).

Commits:
master: 91e0013
Branch_2_2: 9573cae

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

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

resolution: Fixed [ 1 ]

@firebird-automations
Copy link
Author

Commented by: @mrotteveel

On further examination the parser intentionally ignores parser errors so it only has to process the first part of the query, and doesn't need to process the full statement.

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

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

resolution: Fixed [ 1 ] =>

@firebird-automations
Copy link
Author

Commented by: @mrotteveel

Removed check for parser errors, the grammar is incomplete, for example DELETE and UPDATE are only implemented sufficiently to obtain the table name. Now only absence of table name in the statement model is considered for treating it as invalid.

Commits:
master: 047825d
Branch_2_2: 845ea7b

More tests need to be added.

@firebird-automations
Copy link
Author

Commented by: @mrotteveel

Added more tests for the parser:

Commits:
master: ab19e0c
Branch_2_2: 4ccd923

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

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

resolution: Fixed [ 1 ]

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment