davsclaus commented on code in PR #29282:
URL: https://github.com/apache/camel/pull/29282#discussion_r4490077025
##########
components/camel-jdbc/src/main/java/org/apache/camel/component/jdbc/DefaultJdbcPrepareStatementStrategy.java:
##########
@@ -118,10 +119,26 @@ public void remove() {
}
} else {
- // just use a regular iterator
- return
exchange.getContext().getTypeConverter().convertTo(Iterator.class,
- headerMap != null ? headerMap.values() : null);
+ // plain ? placeholders are bound by position, so the values are
only taken from the JDBC_PARAMETERS header
+ // (and not from all the message headers, as their order does not
relate to the query)
+ return createPositionalIterator(query, exchange);
+ }
+ }
+
+ private Iterator<?> createPositionalIterator(String query, Exchange
exchange) {
+ Object parameters =
exchange.getIn().getHeader(JdbcConstants.JDBC_PARAMETERS);
+ if (parameters == null) {
+ throw new RuntimeExchangeException(
+ "Cannot set the ? placeholders in query [" + query + "]:
set their values, in order, as a List in the "
+ + JdbcConstants.JDBC_PARAMETERS
+ + " header, or use named
parameters (:?name)",
+ exchange);
+ } else if (parameters instanceof Map<?, ?> map) {
+ return map.values().iterator();
}
+ // a List, Iterable or array gives its values in order, and any other
value (a String is not split) is
+ // the one value
+ return ObjectHelper.createIterator(parameters, null, true);
Review Comment:
Fixed in 8a79b6333e3f. The positional path no longer uses
`ObjectHelper.createIterator`: only a `Map`, an `Iterable` or an `Object[]`
gives several values, and any other value (a `byte[]`, a `String`, ...) is one
value. `testParameterizedQueryNoNamesByteArray` inserts a `byte[]` into a
`varbinary` column and selects it back with `where data = ?` using the same
`byte[]`.
_Claude Code on behalf of davsclaus_
##########
components/camel-jdbc/src/main/java/org/apache/camel/component/jdbc/DefaultJdbcPrepareStatementStrategy.java:
##########
@@ -118,10 +119,26 @@ public void remove() {
}
} else {
- // just use a regular iterator
- return
exchange.getContext().getTypeConverter().convertTo(Iterator.class,
- headerMap != null ? headerMap.values() : null);
+ // plain ? placeholders are bound by position, so the values are
only taken from the JDBC_PARAMETERS header
+ // (and not from all the message headers, as their order does not
relate to the query)
+ return createPositionalIterator(query, exchange);
+ }
+ }
+
+ private Iterator<?> createPositionalIterator(String query, Exchange
exchange) {
Review Comment:
Done as you proposed, in 8a79b6333e3f: `createPositionalIterator` now gets
`expectedParams` and fails with `Query [...] has N ? placeholders but the
CamelJdbcParameters header has M values` when the counts differ (more or
fewer). `populateStatement` is unchanged. Tests:
`testParameterizedQueryNoNamesTooManyValues`,
`testParameterizedQueryNoNamesTooFewValues`, and
`testParameterizedQueryNoNamesSingleValueWithComma` now checks the new message.
The upgrade guide entry and the component doc say so.
_Claude Code on behalf of davsclaus_
##########
components/camel-jdbc/src/main/java/org/apache/camel/component/jdbc/DefaultJdbcPrepareStatementStrategy.java:
##########
@@ -55,19 +56,19 @@ public Iterator<?> createPopulateIterator(
final String query, final String preparedQuery, final int
expectedParams,
final Exchange exchange, final Object value)
throws SQLException {
- Map<?, ?> map = null;
- if (exchange.getIn().hasHeaders()) {
- if (exchange.getIn().getHeader(JdbcConstants.JDBC_PARAMETERS) !=
null) {
- // header JDBC_PARAMETERS takes precedence over regular headers
- map =
exchange.getIn().getHeader(JdbcConstants.JDBC_PARAMETERS, Map.class);
- } else {
- map = exchange.getIn().getHeaders();
+ if (hasNamedParameters(query)) {
+ Map<?, ?> map = null;
+ if (exchange.getIn().hasHeaders()) {
+ if (exchange.getIn().getHeader(JdbcConstants.JDBC_PARAMETERS)
!= null) {
+ // header JDBC_PARAMETERS takes precedence over regular
headers
+ map =
exchange.getIn().getHeader(JdbcConstants.JDBC_PARAMETERS, Map.class);
Review Comment:
Fixed in 8a79b6333e3f with the fallback: on the named path,
`CamelJdbcParameters` is used only when it is a `Map`; otherwise the values
come from the message headers, as when the header is not set. A non-Map value
has no names, so ignoring it for named parameters seemed less surprising than
failing, and it keeps a route that mixes the two query styles working without
removing the header in between. `testPlainPlaceholdersThenNamedParameters` is
your two-call route. The doc and the upgrade guide entry describe it (before,
such a header made the lookup fail with `Cannot find key`).
_Claude Code on behalf of davsclaus_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
