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

Use JDBC driver-provided named prepared statements if available #32798

Closed
vlsi opened this issue May 11, 2024 · 11 comments
Closed

Use JDBC driver-provided named prepared statements if available #32798

vlsi opened this issue May 11, 2024 · 11 comments
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: declined A suggestion or change that we don't feel we should currently apply

Comments

@vlsi
Copy link
Contributor

vlsi commented May 11, 2024

This is to discuss an enhancement.

Problem

PostgreSQL JDBC driver team discusses an enhancement so the driver could support named parameters via non-standard API.
By default, JDBC provides positional arguments only, so people can't use queries like name = :name or :name is null, and they have to pass :name twice.

In fact, PostgreSQL does support "reusing placeholder value several times", so PostgreSQL-native query could be like name = $1 or $1 is null where $1 would be a single parameter. In that case, JDBC driver would pass the value for $1 just once, and the database would reuse it at both positions.

So here's my question: what do you think of using pgjdbc-specific extensions within spring-framework?
For instance, what if NamedParameterJdbcTemplate (or the relevant bit) would use special API so it could defer named parameter transformation to the JDBC driver?

Here's pgjdbc PR: pgjdbc/pgjdbc#1946

Examples

Here's a draft API:

// The use of try-with-resources for switching placeholder parsing mode enables structured operation,
// So the placeholder mode is never "leaked out of the intended scope"
try (
    // Switch the connection to "named" placeholders style
    Connection con2 = con.unwrap(PGConnection.class).withPlaceholderStyle(PlaceholderStyle.NAMED);
    PreparedStatement ps = con.prepareStatement("SELECT col_a FROM mytable WHERE col_a < :myParam AND col_b > :myParam");
) {
    PGPreparedStatement pps = ps.unwrap(PGPreparedStatement.class);
    // Here's how we bind the named parameter
    pps.setIntAtName("myParam", 42);
    try (ResultSet rs = ps.executeQuery(); ) {
        // ...
    }
}

An alternative option could be driver-specific API to prepare statements like:

con.unwrap(PGConnection.class)
    .prepareStatement("SELECT col_a FROM mytable WHERE col_a < :myParam AND col_b > :myParam", PlaceholderStyle.NAMED)

Note: JDBC specifies 6 prepareStatement methods (e.g. autogeneratedKeys, resultsettype, holdability, and so on), so if we go with adding PlaceholderStyle parameter, we would have to add 6 new prepareStatement methods.

The native PostgreSQL syntax is to use $1, ... $99 variable names, so an alternative option for NamedParameterJdbcTemplate could be replacing its names with $n names. I am not sure it makes a huge difference though.

WDYT?


Oracle DB JDBC does have the similar API for quite some time now (I guess 10+ years): OraclePreparedStatement.setIntAtName(String, int)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 11, 2024
@snicoll snicoll changed the title Enhancement: use JDBC driver-provided named prepared statements if avaliable Use JDBC driver-provided named prepared statements if available May 11, 2024
@snicoll snicoll added the in: data Issues in data modules (jdbc, orm, oxm, tx) label May 11, 2024
@quaff
Copy link
Contributor

quaff commented May 12, 2024

Note: JDBC specifies 6 prepareStatement methods (e.g. autogeneratedKeys, resultsettype, holdability, and so on), so if we go with adding PlaceholderStyle parameter, we would have to add 6 new prepareStatement methods.

It would be nice if the driver auto detect named or positional parameters since mixing them is forbidden, like JPA does.

@vlsi
Copy link
Contributor Author

vlsi commented May 12, 2024

Based on my current understanding, "auto detect" is likely not an option as it might create subtle parsing issues.
For instance, with "positional" (JDBC) style, one has to duplicate ? if they want a single ? in the SQL.

Imagine somebody wants executing SQL that has no parameters yet they want including ? as PostgreSQL's JSON operator. Then "autodetect" won't be able to tell if ? was meant for a placeholder or if it was just a PG operator.

@quaff
Copy link
Contributor

quaff commented May 12, 2024

Based on my current understanding, "auto detect" is likely not an option as it might create subtle parsing issues. For instance, with "positional" (JDBC) style, one has to duplicate ? if they want a single ? in the SQL.

Imagine somebody wants executing SQL that has no parameters yet they want including ? as PostgreSQL's JSON operator. Then "autodetect" won't be able to tell if ? was meant for a placeholder or if it was just a PG operator.

If there is any named parameter like :xxx then it should be PlaceholderStyle.NAMED, otherwise it's positional.

@vlsi
Copy link
Contributor Author

vlsi commented May 12, 2024

If there is any named parameter like :xxx then it should be PlaceholderStyle.NAMED, otherwise it's positional.

The query might contain no placeholders at all in which case you won't be able to tell if it is "positional or named".

PostgreSQL has Does the key/element string exist within the JSON value? '{"a":1, "b":2}'::jsonb ? 'b'.
The user might assume they use "named" while this ? would accidentally be treated as if it was a placeholder.
Such "automatic" detection would effectively bring the worse of the two worlds, so we should not implement it.

@quaff
Copy link
Contributor

quaff commented May 12, 2024

The query might contain no placeholders at all in which case you won't be able to tell if it is "positional or named".

Then use positional as default because it's JDBC standard.

The user might assume they use "named" while this ? would accidentally be treated as if it was a placeholder.

The user should know it's named parameter query because they write sql to include :name.

@vlsi
Copy link
Contributor Author

vlsi commented May 12, 2024

Please, this is getting off-topic. The key question for this issue is whether spring-framework is willing to use non-standard extensions to JDBC provided by the drivers

@snicoll
Copy link
Member

snicoll commented May 21, 2024

Thanks for the suggestion but NamedParameterJdbcTemplate is meant to parse Spring-level names into positional parameters for the driver, and its parsing logic is already complex enough as it is.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale May 21, 2024
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 21, 2024
@snicoll
Copy link
Member

snicoll commented May 21, 2024

@vlsi I don't know what the confused emoji means but we're not restricting things in any way for those who are willing to use the PreparedStatement the way postgres supports it. NamedParameterJdbcTemplate is a Spring-specific feature and we can justify accommodate with all the subtle variant of all database drivers at that level.

@vlsi
Copy link
Contributor Author

vlsi commented May 21, 2024

Consider the following:

  • Even though Spring might have its own "SQL syntax", it is doomed to send repeated variables several times. For instance, if the query has something like description = :var or name = :var or somethingelse = :var, then Spring would convert it to three different JDBC placeholders, and it would force the driver to pass the same value several times.
    If Spring used "named JDBC placeholders", then the driver could reuse the value and pass it just once over the wire. It would reduce the network traffic, and improve decoding as the database would not have to parse the parameter several times
  • Currently, Spring has to implement "SQL with named parameters" -> "JDBC-style SQL". If you use driver-provided APIs, then you do not need caching at the Spring level since the driver would cache anyway. If you miss such a cache, then you effectively have to parse and build "JDBC SQL" every time which is a huge resource waste.

In other words, leveraging driver API would improve performance and reduce CPU/memory utilization.

NamedParameterJdbcTemplate is a Spring-specific

Do you have Spring-specific SQL syntax there? If the only difference is "parameters should be :named", then it is the same that Oracle DB provides, and the same might be considered for pgjdbc.

@vlsi
Copy link
Contributor Author

vlsi commented May 21, 2024

@snicoll , I understand NamedParameterJdbcTemplate is a Spring-specific class.
However, I do not fully understand reasons for declining a proposal of improving NamedParameterJdbcTemplate by leveraging driver features.

For instance, Java provides java.util.concurrent.ConcurrentHashMap and you use it in spring-framework even in the case you could write your own implementation. The same goes for JDBC: what would be the reasons for not using the features that drivers provide?

@jhoeller
Copy link
Contributor

@vlsi we appreciate the intention there but unless this becomes a standard JDBC feature - and there ideally even in our JDBC baseline version -, we would effectively introduce separate code paths for processing a named parameter query against new-enough versions of the Postgres and Oracle drivers, with unwrapping the PreparedStatement from the connection pool handle down to the native driver first, and potentially with subtle differences in parsing/encoding/etc next to our regular code path where we parse a named parameter query into positional parameters ourselves. For the time being, the maintenance impact of such an arrangement is not desirable for us at all. We would only really embrace such driver-level parsing if we could revise our entire arrangement towards standard driver support there, getting rid of our own named parameter parsing completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

5 participants