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

Refactor implementation of {call ...} escape for better support in Statement, PreparedStatement and CallableStatement [JDBC296] #342

Open
firebird-automations opened this issue Jan 3, 2013 · 14 comments

Comments

@firebird-automations
Copy link

Submitted by: @mrotteveel

Is related to JDBC292
Relate to JDBC229
Relate to JDBC297
Relate to JDBC402

The {call ...}-escape should be supported on Statement, PreparedStatement and CallableStatement (in various degrees), the current implementation is inefficient for CallableStatement (it parses the statement two or three times) and it looks like it might shuffle the parameter order in the case of {?= call ...} (not 100% sure), and support under Statement and PreparedStatement seems accidental (and there are no tests).

Also the existing implementation considers parentheses around the parameter list to be optional; they are required by the syntax defined in section 13.4.4 of the JDBC spec (they are only optional if there are no parameters).

See also http://mail.openjdk.java.net/pipermail/jdbc-spec-discuss/2013-January/000029.html

================================
ORIGINAL TEXT:
Currently the {call ... } escape is supported for Statement and PreparedStatement as well. It should only be supported for CallableStatement. This will also reduce the complexity of parsing (with repeated parsing) as described in JDBC292.

Implementation-wise the JDBCEscape parser should be changed to simply leave the {call ...} and {? = call ...} escapes untouched (but it should parse the escapes nested inside the call, leaving it upto Firebird to throw an excepion when it receives {call ...} untranslated. Then only the FBCallableStatement/FBEscapedCallParser implementation should translate the call escape to the appropriate Firebird specific call.

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

Fix Version: Jaybird 2.3 [ 10440 ]

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

description: Currently the {call ... } escape is supported for Statement and PreparedStatement as well. It should only be supported for CallableStatement. This will also reduce the complexity of parsing (with repeated parsing) as described in JDBC292.

Implementation-wise the JDBCEscape parser should be changed to simply leave the {call ...} and {? = call ...} escapes untouched (but it should parse the escapes nested inside the call, leaving it upto Firebird to throw an excepion when it receives {call ...} untranslated. Then only the FBCallableStatement/FBEscapedCallParser implementation should translate the call escape to the appropriate escape.

=>

Currently the {call ... } escape is supported for Statement and PreparedStatement as well. It should only be supported for CallableStatement. This will also reduce the complexity of parsing (with repeated parsing) as described in JDBC292.

Implementation-wise the JDBCEscape parser should be changed to simply leave the {call ...} and {? = call ...} escapes untouched (but it should parse the escapes nested inside the call, leaving it upto Firebird to throw an excepion when it receives {call ...} untranslated. Then only the FBCallableStatement/FBEscapedCallParser implementation should translate the call escape to the appropriate Firebird specific call.

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

Link: This issue is related to JDBC292 [ JDBC292 ]

@firebird-automations
Copy link
Author

Commented by: @mrotteveel

Looking at section 6.4 of JDBC 4.1, it seems that the {call...} should also be supported on Statement and PreparedStatement. This means I have to find a different way of preventing double parsing when CallableStatement is used....

@firebird-automations
Copy link
Author

Commented by: @mrotteveel

Sent a mail to jdbc-spec-discuss to ask whether the call-escape should be supported for Statement and PreparedStatement.

@firebird-automations
Copy link
Author

Commented by: @mrotteveel

Statement and PreparedStatement should support the {call } escape as well. For Statement only with literal parameters, for PreparedStatement only with IN parameters, and both don't need to support OUT parameters in the JDBC sense (but - of course - have to support ResultSet producing stored procedures).

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

description: Currently the {call ... } escape is supported for Statement and PreparedStatement as well. It should only be supported for CallableStatement. This will also reduce the complexity of parsing (with repeated parsing) as described in JDBC292.

Implementation-wise the JDBCEscape parser should be changed to simply leave the {call ...} and {? = call ...} escapes untouched (but it should parse the escapes nested inside the call, leaving it upto Firebird to throw an excepion when it receives {call ...} untranslated. Then only the FBCallableStatement/FBEscapedCallParser implementation should translate the call escape to the appropriate Firebird specific call.

=>

The {call ...}-escape should be supported on Statement, PreparedStatement and CallableStatement (in various degrees), the current implementation is inefficient for CallableStatement (it parses the statement two or three times) and it looks like it might shuffle the parameter order in the case of {?= call ...} (not 100% sure), and support under Statement and PreparedStatement seems accidental (and there are no tests).

Also the existing implementation considers parentheses around the parameter list to be optional; they are required by the syntax defined in section 13.4.4 of the JDBC spec (they are only optional if there are no parameters).

See also http://mail.openjdk.java.net/pipermail/jdbc-spec-discuss/2013-January/000029.html

================================
ORIGINAL TEXT:
Currently the {call ... } escape is supported for Statement and PreparedStatement as well. It should only be supported for CallableStatement. This will also reduce the complexity of parsing (with repeated parsing) as described in JDBC292.

Implementation-wise the JDBCEscape parser should be changed to simply leave the {call ...} and {? = call ...} escapes untouched (but it should parse the escapes nested inside the call, leaving it upto Firebird to throw an excepion when it receives {call ...} untranslated. Then only the FBCallableStatement/FBEscapedCallParser implementation should translate the call escape to the appropriate Firebird specific call.

summary: Refactor support for {call ...} escape support so it can no longer be used from Statement and PreparedStatement. => Refactor support for {call ...} escape support for better support in Statement, PreparedStatement and CallableStatement

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

summary: Refactor support for {call ...} escape support for better support in Statement, PreparedStatement and CallableStatement => Refactor implementation of {call ...} escape for better support in Statement, PreparedStatement and CallableStatement

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

Link: This issue relate to JDBC229 [ JDBC229 ]

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

Link: This issue relate to JDBC297 [ JDBC297 ]

@firebird-automations
Copy link
Author

Commented by: @mrotteveel

See also JDBC402, one thing to check for is the validity of the current double parsing of the call escape, and whether the effect of FBProcedureCall.checkParameters() in FBEscapedParser.convertProcedureCall is actually the correct one.

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

Link: This issue relate to JDBC402 [ JDBC402 ]

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

Fix Version: Jaybird 3.1 [ 10441 ]

Fix Version: Jaybird 3.0 [ 10440 ] =>

@firebird-automations
Copy link
Author

Modified by: @mrotteveel

Fix Version: Jaybird 5 [ 10871 ]

Fix Version: Jaybird 4 [ 10441 ] =>

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

No branches or pull requests

2 participants