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-issue-importer opened this issue Apr 20, 2015 · 10 comments

Comments

@firebird-issue-importer

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 20, 2015

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 20, 2015

Commented by: @mrotteveel

Added initial test that demonstrates this is broken.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 27, 2015

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 27, 2015

Modified by: @mrotteveel

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

resolution: Fixed [ 1 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 27, 2015

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 27, 2015

Modified by: @mrotteveel

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

resolution: Fixed [ 1 ] =>

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 27, 2015

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 2, 2015

Commented by: @mrotteveel

Added more tests for the parser:

Commits:
master: ab19e0c
Branch_2_2: 4ccd923

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 2, 2015

Modified by: @mrotteveel

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

resolution: Fixed [ 1 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 31, 2015

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