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

Parameter parsing for PreparedStatement #9

Open
SeriyBg opened this issue Dec 16, 2020 · 4 comments
Open

Parameter parsing for PreparedStatement #9

SeriyBg opened this issue Dec 16, 2020 · 4 comments
Assignees

Comments

@SeriyBg
Copy link
Contributor

SeriyBg commented Dec 16, 2020

For now any ? is considered as a parameter that is not a correct behavior as ? might be present in the string literal (SELECT * FROM foo WHERE bar='?' AND baz= ?).
Also, we need to validate the correctness of the parameter location, e.g. SELECT * FROM ? WHERE foo='bar' is not valid.

@SeriyBg SeriyBg added this to the 4.2 milestone Dec 16, 2020
@SeriyBg SeriyBg self-assigned this Dec 16, 2020
@SeriyBg SeriyBg removed this from the 4.2 milestone Dec 29, 2020
@viliam-durina
Copy link
Contributor

IMO this should be handled by server and that it already is. The driver should send the sql string and params as is to the server.

@SeriyBg
Copy link
Contributor Author

SeriyBg commented Feb 26, 2021

@viliam-durina according to the JavaDoc of setter methods it should validate the parameters. For example for the query SELECT * FROM foo WHERE bar=? calling statement.setInt(2, 3) should throw an exception. However, the JDBC is calling the server on execute after setting the parameter. We decided not to implement it for now, as won't bring much value but will require a lot of effort. But we may need to implement it in the future

@viliam-durina
Copy link
Contributor

I tested postgresql:

PreparedStatement st = conn.prepareStatement("select * from t");
st.setInt(1, 1); // throws

st = conn.prepareStatement("select * from ?");
st.setInt(1, 1); // does not throw

st = conn.prepareStatement("select * from m /* where a=? */");
st.setInt(1, 1); // throws

// this fails at parsing
st = conn.prepareStatement("select * from m /* where a=?");

This is their parser: org.postgresql.core.Parser.parseSql(), it's quite large, but it's still best-effort, see the 2nd query. IMO it's fine to always throw in execute.

@SeriyBg
Copy link
Contributor Author

SeriyBg commented Feb 26, 2021

The second one should also throw an exception at the execute. Agree that we don't need to invest in it, at least for now

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

No branches or pull requests

2 participants