diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/CQL4SkipMetadataResolveMethod.java b/core/src/main/java/com/datastax/oss/driver/api/core/CQL4SkipMetadataResolveMethod.java index 723f2749a9a..c586851e50a 100644 --- a/core/src/main/java/com/datastax/oss/driver/api/core/CQL4SkipMetadataResolveMethod.java +++ b/core/src/main/java/com/datastax/oss/driver/api/core/CQL4SkipMetadataResolveMethod.java @@ -1,6 +1,7 @@ package com.datastax.oss.driver.api.core; import edu.umd.cs.findbugs.annotations.NonNull; +import java.util.Locale; public enum CQL4SkipMetadataResolveMethod { // SMART (Default) - Disables the skip metadata flag only for wildcard selects (`SELECT * FROM`) @@ -28,7 +29,9 @@ public String toString() { @NonNull public static CQL4SkipMetadataResolveMethod fromValue(@NonNull String value) throws IllegalArgumentException { - switch (value.toLowerCase()) { + // ROOT, not the default locale: in a Turkish JVM the I of "DISABLED" folds to a dotless + // small letter, which matches no case below and rejects a valid configuration. + switch (value.toLowerCase(Locale.ROOT)) { case "smart": return SMART; case "enabled": diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java b/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java index d73d7dd2bb5..d896d06e02b 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java @@ -69,6 +69,7 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -177,7 +178,9 @@ private RequestRoutingMethod parseLwtRequestRoutingMethod() { return RequestRoutingMethod.PRESERVE_REPLICA_ORDER; } try { - return RequestRoutingMethod.valueOf(methodString.toUpperCase()); + // ROOT, not the default locale: in a Turkish JVM the i of "preserve_replica_order" folds + // to a dotted capital, so a valid setting would be warned about and silently dropped. + return RequestRoutingMethod.valueOf(methodString.toUpperCase(Locale.ROOT)); } catch (IllegalArgumentException e) { LOG.warn( "[{}] Unknown request routing method '{}', defaulting to PRESERVE_REPLICA_ORDER", diff --git a/core/src/test/java/com/datastax/oss/driver/api/core/CQL4SkipMetadataResolveMethodTest.java b/core/src/test/java/com/datastax/oss/driver/api/core/CQL4SkipMetadataResolveMethodTest.java new file mode 100644 index 00000000000..12663a1fd3e --- /dev/null +++ b/core/src/test/java/com/datastax/oss/driver/api/core/CQL4SkipMetadataResolveMethodTest.java @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datastax.oss.driver.api.core; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.datastax.oss.driver.TestDataProviders; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.util.Locale; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(DataProviderRunner.class) +public class CQL4SkipMetadataResolveMethodTest { + + /** + * {@code fromValue} folds the configured value to compare it, so it must pin {@link Locale#ROOT}: + * the I of {@code DISABLED} and {@code ENABLED} folds to a dotless small letter in a Turkish JVM, + * which would match none of the cases and reject a valid configuration at session build. + */ + @Test + @UseDataProvider(location = TestDataProviders.class, value = "locales") + public void should_parse_value_in_any_default_locale(Locale locale) { + Locale def = Locale.getDefault(); + try { + Locale.setDefault(locale); + assertThat(CQL4SkipMetadataResolveMethod.fromValue("DISABLED")) + .isEqualTo(CQL4SkipMetadataResolveMethod.DISABLED); + assertThat(CQL4SkipMetadataResolveMethod.fromValue("ENABLED")) + .isEqualTo(CQL4SkipMetadataResolveMethod.ENABLED); + assertThat(CQL4SkipMetadataResolveMethod.fromValue("SMART")) + .isEqualTo(CQL4SkipMetadataResolveMethod.SMART); + assertThat(CQL4SkipMetadataResolveMethod.fromValue("disabled")) + .isEqualTo(CQL4SkipMetadataResolveMethod.DISABLED); + } finally { + Locale.setDefault(def); + } + } + + @Test + public void should_fail_to_parse_unknown_value() { + assertThatThrownBy(() -> CQL4SkipMetadataResolveMethod.fromValue("nope")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Unsupported value nope"); + } +} diff --git a/core/src/test/java/com/datastax/oss/driver/api/core/CqlIdentifierTest.java b/core/src/test/java/com/datastax/oss/driver/api/core/CqlIdentifierTest.java index 5c7203b8f8d..6016fbec0d9 100644 --- a/core/src/test/java/com/datastax/oss/driver/api/core/CqlIdentifierTest.java +++ b/core/src/test/java/com/datastax/oss/driver/api/core/CqlIdentifierTest.java @@ -67,6 +67,17 @@ public void should_fail_to_build_from_valid_cql_if_reserved_keyword() { CqlIdentifier.fromCql("Create"); } + /** + * Servers synthesize names like {@code IN(ck)} for anonymous markers of an IN relation. They can + * only be turned into an identifier through {@link CqlIdentifier#fromInternal} or a double-quoted + * CQL form, which is one of the reasons applications should bind such markers positionally + * instead. See the prepared-statements manual page. + */ + @Test(expected = IllegalArgumentException.class) + public void should_fail_to_build_from_valid_cql_if_synthesized_marker_name() { + CqlIdentifier.fromCql("IN(ck)"); + } + @Test public void should_format_as_cql() { assertThat(CqlIdentifier.fromInternal("foo").asCql(false)).isEqualTo("\"foo\""); diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/cql/ConversionsTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/cql/ConversionsTest.java index 954cf0e14a0..e06123c945b 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/cql/ConversionsTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/cql/ConversionsTest.java @@ -18,14 +18,29 @@ package com.datastax.oss.driver.internal.core.cql; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.datastax.oss.driver.api.core.DefaultProtocolVersion; +import com.datastax.oss.driver.api.core.config.DefaultDriverOption; +import com.datastax.oss.driver.api.core.config.DriverExecutionProfile; +import com.datastax.oss.driver.api.core.cql.BoundStatement; import com.datastax.oss.driver.api.core.cql.ColumnDefinition; import com.datastax.oss.driver.api.core.cql.ColumnDefinitions; +import com.datastax.oss.driver.api.core.cql.PreparedStatement; +import com.datastax.oss.driver.api.core.cql.Statement; import com.datastax.oss.driver.api.core.metadata.schema.ColumnMetadata; +import com.datastax.oss.driver.api.core.time.TimestampGenerator; +import com.datastax.oss.driver.api.core.type.codec.registry.CodecRegistry; +import com.datastax.oss.driver.internal.core.DefaultConsistencyLevelRegistry; +import com.datastax.oss.driver.internal.core.ProtocolVersionRegistry; +import com.datastax.oss.driver.internal.core.context.InternalDriverContext; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; +import com.datastax.oss.protocol.internal.Message; +import com.datastax.oss.protocol.internal.request.Execute; +import java.nio.ByteBuffer; import java.util.List; import org.junit.Test; @@ -87,4 +102,60 @@ private ColumnDefinitions variables(String... columnNames) { } return DefaultColumnDefinitions.valueOf(columns.build()); } + + /** + * The invariant that makes the driver immune to CUSTOMER-583: an EXECUTE carries its values + * positionally, so the name the server synthesized for an anonymous marker never travels back to + * the coordinator and cannot be re-resolved there. Everything else in this area only guards the + * local name-to-index lookup; if this branch ever started sending named values, that lookup would + * be bypassed and a server that respelled a marker between PREPARE and EXECUTE would break + * applications again. + */ + @Test + public void should_send_bound_statement_values_positionally() { + List values = + ImmutableList.of(ByteBuffer.allocate(1), ByteBuffer.allocate(2), ByteBuffer.allocate(3)); + + Message message = Conversions.toMessage(boundStatement(values), profile(), context()); + + assertThat(message).isInstanceOf(Execute.class); + Execute execute = (Execute) message; + assertThat(execute.options.namedValues).isEmpty(); + assertThat(execute.options.positionalValues).isEqualTo(values); + } + + private BoundStatement boundStatement(List values) { + // Built before the stubbing below: variables() mocks in turn, and nesting that inside a when() + // argument leaves Mockito with an unfinished stubbing. + ColumnDefinitions resultSetDefinitions = variables("v"); + PreparedStatement preparedStatement = mock(PreparedStatement.class); + when(preparedStatement.getId()).thenReturn(ByteBuffer.allocate(4)); + when(preparedStatement.getResultSetDefinitions()).thenReturn(resultSetDefinitions); + BoundStatement boundStatement = mock(BoundStatement.class); + when(boundStatement.getPreparedStatement()).thenReturn(preparedStatement); + when(boundStatement.getValues()).thenReturn(values); + when(boundStatement.getQueryTimestamp()).thenReturn(Statement.NO_DEFAULT_TIMESTAMP); + when(boundStatement.getNowInSeconds()).thenReturn(Statement.NO_NOW_IN_SECONDS); + return boundStatement; + } + + private DriverExecutionProfile profile() { + DriverExecutionProfile profile = mock(DriverExecutionProfile.class); + when(profile.getString(DefaultDriverOption.REQUEST_CONSISTENCY)).thenReturn("LOCAL_ONE"); + when(profile.getInt(DefaultDriverOption.REQUEST_PAGE_SIZE)).thenReturn(5000); + when(profile.getString(DefaultDriverOption.REQUEST_SERIAL_CONSISTENCY)).thenReturn("SERIAL"); + return profile; + } + + private InternalDriverContext context() { + ProtocolVersionRegistry protocolVersionRegistry = mock(ProtocolVersionRegistry.class); + when(protocolVersionRegistry.supports(any(), any())).thenReturn(true); + InternalDriverContext context = mock(InternalDriverContext.class); + when(context.getConsistencyLevelRegistry()).thenReturn(new DefaultConsistencyLevelRegistry()); + when(context.getTimestampGenerator()).thenReturn(mock(TimestampGenerator.class)); + when(context.getCodecRegistry()).thenReturn(CodecRegistry.DEFAULT); + when(context.getProtocolVersion()).thenReturn(DefaultProtocolVersion.V4); + when(context.getProtocolVersionRegistry()).thenReturn(protocolVersionRegistry); + return context; + } } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/data/IdentifierIndexTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/data/IdentifierIndexTest.java index 697a32fb029..1265c0b78b8 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/data/IdentifierIndexTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/data/IdentifierIndexTest.java @@ -36,6 +36,26 @@ public class IdentifierIndexTest { private IdentifierIndex index = new IdentifierIndex(ImmutableList.of(Foo, foo, fOO, Foo, foo, fOO)); + // The variable definitions a server returns for + // "SELECT * FROM t WHERE pk = ? AND ck IN ? AND ck IN ?": the markers of the IN relations get a + // name synthesized from the operator and the column, so repeating the column yields the same name + // twice. That spelling differs between ScyllaDB release lines rather than along a single version + // sequence: 2024.1 emits in(ck), 2026.1.8 emits IN(ck), and the lowercase spelling is restored in + // 2026.1.12 and 2026.2.6 (CUSTOMER-583 / SCYLLADB-3454). An application must therefore not depend + // on either spelling. See BoundStatementCcmIT for the end-to-end counterpart of the tests below. + private static final CqlIdentifier pk = CqlIdentifier.fromInternal("pk"); + private static final CqlIdentifier upperCaseInCk = CqlIdentifier.fromInternal("IN(ck)"); + + /** + * Built on demand rather than kept in a field: the constructor folds the names it indexes, so the + * locale test below has to build the index inside its own locale override to cover that half of + * the fold. A field initializer would run before the override and leave the indexing side + * untested. + */ + private static IdentifierIndex synthesizedIndex() { + return new IdentifierIndex(ImmutableList.of(pk, upperCaseInCk, upperCaseInCk)); + } + @Test public void should_find_first_index_of_existing_identifier() { assertThat(index.firstIndexOf(Foo)).isEqualTo(0); @@ -139,4 +159,50 @@ public void should_find_all_indices_of_case_sensitive_name() { public void should_not_find_indices_of_nonexistent_case_sensitive_name() { assertThat(index.allIndicesOf("\"FOO\"")).isEmpty(); } + + /** + * The regression guard for CUSTOMER-583: the driver resolves bind-variable names locally, and + * that lookup must not care how the server spelled a synthesized name. The locales provider + * matters here rather than incidentally: the letter that flipped is {@code I}, and lowercasing it + * in the Turkish locale yields a dotless {@code ı}, so a lookup that did not pin {@link + * Locale#ROOT} would fail for Turkish users only. + */ + @Test + @UseDataProvider(location = TestDataProviders.class, value = "locales") + public void should_find_synthesized_marker_name_whatever_the_server_spelling(Locale locale) { + Locale def = Locale.getDefault(); + try { + Locale.setDefault(locale); + // Built inside the override deliberately, so that the fold the constructor applies to the + // names it indexes is exercised under this locale too, not just the fold on the lookup side. + IdentifierIndex synthesized = synthesizedIndex(); + assertThat(synthesized.firstIndexOf("IN(ck)")).isEqualTo(1); + assertThat(synthesized.firstIndexOf("in(ck)")).isEqualTo(1); + assertThat(synthesized.firstIndexOf("In(Ck)")).isEqualTo(1); + } finally { + Locale.setDefault(def); + } + } + + /** A named setter writes every matching variable, so repeating a column makes names ambiguous. */ + @Test + public void should_find_all_indices_of_synthesized_marker_name_when_column_is_repeated() { + assertThat(synthesizedIndex().allIndicesOf("in(ck)")).containsExactly(1, 2); + } + + /** Double-quoting opts into exact matching, which the synthesized spelling can then break. */ + @Test + public void should_match_double_quoted_synthesized_marker_name_exactly() { + IdentifierIndex synthesized = synthesizedIndex(); + assertThat(synthesized.firstIndexOf("\"IN(ck)\"")).isEqualTo(1); + assertThat(synthesized.firstIndexOf("\"in(ck)\"")).isEqualTo(-1); + } + + /** Same for the identifier-based lookup, which is always exact. */ + @Test + public void should_match_synthesized_marker_identifier_exactly() { + IdentifierIndex synthesized = synthesizedIndex(); + assertThat(synthesized.firstIndexOf(CqlIdentifier.fromInternal("IN(ck)"))).isEqualTo(1); + assertThat(synthesized.firstIndexOf(CqlIdentifier.fromInternal("in(ck)"))).isEqualTo(-1); + } } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyConfigTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyConfigTest.java index 768722e0e86..2e90e0c26c5 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyConfigTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyConfigTest.java @@ -28,12 +28,15 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import ch.qos.logback.classic.spi.ILoggingEvent; +import com.datastax.oss.driver.TestDataProviders; import com.datastax.oss.driver.api.core.config.DefaultDriverOption; import com.datastax.oss.driver.api.core.config.DriverExecutionProfile; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.util.Locale; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -104,6 +107,36 @@ public void should_accept_configuration_combinations( .getBoolean(DefaultDriverOption.LOAD_BALANCING_POLICY_SLOW_AVOIDANCE, true); } + /** + * The configured value is upper-cased before {@code valueOf}, so the fold must pin {@link + * Locale#ROOT}: in a Turkish JVM the i of {@code preserve_replica_order} becomes a dotted + * capital, {@code valueOf} then fails, and the policy silently falls back to the value the user + * had asked for anyway — observable only as this warning. The fallback is why the assertion has + * to be on the log rather than on the resolved method. + */ + @Test + @UseDataProvider(location = TestDataProviders.class, value = "locales") + public void should_accept_valid_routing_method_in_any_default_locale(Locale locale) { + when(metadataManager.getContactPoints()).thenReturn(ImmutableSet.of(node1)); + when(defaultProfile.getString( + DefaultDriverOption.LOAD_BALANCING_DEFAULT_LWT_REQUEST_ROUTING_METHOD)) + .thenReturn("preserve_replica_order"); + + Locale def = Locale.getDefault(); + try { + Locale.setDefault(locale); + assertThat(new DefaultLoadBalancingPolicy(context, DriverExecutionProfile.DEFAULT_NAME)) + .isNotNull(); + } finally { + Locale.setDefault(def); + } + + verify(appender, atLeast(0)).doAppend(loggingEventCaptor.capture()); + assertThat(loggingEventCaptor.getAllValues()) + .extracting(ILoggingEvent::getFormattedMessage) + .noneMatch(message -> message.contains("Unknown request routing method")); + } + @DataProvider public static Object[][] configurationCombinations() { return new Object[][] { diff --git a/integration-tests/src/test/java/com/datastax/oss/driver/core/cql/BoundStatementCcmIT.java b/integration-tests/src/test/java/com/datastax/oss/driver/core/cql/BoundStatementCcmIT.java index d1802d0b691..c3520c72734 100644 --- a/integration-tests/src/test/java/com/datastax/oss/driver/core/cql/BoundStatementCcmIT.java +++ b/integration-tests/src/test/java/com/datastax/oss/driver/core/cql/BoundStatementCcmIT.java @@ -73,7 +73,9 @@ import com.tngtech.java.junit.dataprovider.UseDataProvider; import java.nio.ByteBuffer; import java.time.Duration; +import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.concurrent.CompletionStage; import java.util.function.Function; @@ -433,6 +435,66 @@ public void should_set_all_occurrences_of_variable() { should_set_all_occurrences_of_variable(ps.boundStatementBuilder().setInt(id, 12).build()); } + /** + * The driver resolves prepared-statement variable names locally, so it must not depend on the + * name the server synthesizes for an anonymous marker. That name differs between ScyllaDB release + * lines rather than along a single version sequence: 2024.1 spells the marker of an IN relation + * {@code in(v)}, 2026.1.8 spells it {@code IN(v)}, and the lowercase spelling is restored in + * 2026.1.12 and 2026.2.6 (CUSTOMER-583 / SCYLLADB-3454); Apache Cassandra spells it {@code + * in(v)}. This test therefore reads the name back from the metadata instead of hardcoding a + * spelling, and asserts that both cases of it resolve to the same variable — as well as the + * positional binding that the manual recommends applications use instead. + */ + @Test + public void should_bind_anonymous_in_marker_by_position_and_by_either_synthesized_case() { + CqlSession session = sessionRule.session(); + PreparedStatement ps = session.prepare("SELECT v FROM test WHERE k = ? AND v IN ?"); + + ColumnDefinitions variables = ps.getVariableDefinitions(); + assertThat(variables).hasSize(2); + String synthesized = variables.get(1).getName().asInternal(); + + // Skip rather than fail if the server ever names the marker plainly "v": everything below + // still passes then, but it no longer covers the synthesized-name mechanism at all, and the + // spelling is precisely what this test refuses to treat as a contract. + assumeThat(synthesized).as("synthesized marker name").isNotEqualTo("v").contains("("); + + // Whatever the server sent, the case of the name must not decide whether it resolves. + assertThat(variables.firstIndexOf(synthesized)).isEqualTo(1); + assertThat(variables.firstIndexOf(synthesized.toLowerCase(Locale.ROOT))).isEqualTo(1); + assertThat(variables.firstIndexOf(synthesized.toUpperCase(Locale.ROOT))).isEqualTo(1); + + List in = ImmutableList.of(1, 2, 3); + + // What applications should do: fill anonymous markers by position. + assertThat(selectedValues(session, ps.bind().setString(0, KEY).setList(1, in, Integer.class))) + .containsExactlyElementsOf(in); + + // What CUSTOMER-583 did. It works here because the String setters ignore case, but the manual + // steers applications away from it: the spelling is not part of any contract. Only the IN + // marker is addressed by name -- the marker of "k = ?" is bound by position, so the case under + // test is not mixed with a second synthesized name the server could also respell. + for (String spelling : + ImmutableList.of( + synthesized, + synthesized.toLowerCase(Locale.ROOT), + synthesized.toUpperCase(Locale.ROOT))) { + assertThat( + selectedValues( + session, ps.bind().setString(0, KEY).setList(spelling, in, Integer.class))) + .as("bound by name %s", spelling) + .containsExactlyElementsOf(in); + } + } + + private static List selectedValues(CqlSession session, BoundStatement bound) { + List values = new ArrayList<>(); + for (Row row : session.execute(bound)) { + values.add(row.getInt("v")); + } + return values; + } + @DataProvider public static Object[][] cql4SkipMetadataResolveMethod() { return new Object[][] { diff --git a/manual/core/statements/prepared/README.md b/manual/core/statements/prepared/README.md index 5a87b238cbc..9b1a1a75af8 100644 --- a/manual/core/statements/prepared/README.md +++ b/manual/core/statements/prepared/README.md @@ -202,18 +202,54 @@ BoundStatement bound = .build(); ``` -You can use named setters even if the query uses anonymous parameters; Cassandra names the -parameters after the column they apply to: +#### Anonymous markers and server-synthesized names + +Named setters also work when the query uses anonymous `?` markers: the server synthesizes a name for +each one, usually after the column it applies to. ```java +// Works, but relies on a name the server made up: BoundStatement bound = ps1.bind() .setString("sku", "324378") .setString("description", "LCD screen"); ``` -This can be ambiguous if the query uses the same column multiple times, like in `select * from sales -where sku = ? and date > ? and date < ?`. In these situations, use positional setters or named -parameters. +**Avoid relying on this.** Synthesized names are not part of any API contract, and their +spelling differs between server release lines, not just between successive versions. For +`SELECT ... WHERE application_id IN ?`, ScyllaDB's 2024.1 releases name the marker +`in(application_id)`, while 2026.1.8 names it `IN(application_id)`; the lowercase spelling is +restored in 2026.1.12 and 2026.2.6. Apache Cassandra names it `in(application_id)`. The regression +was reported against a driver that matches these names exactly: the hardcoded lowercase spelling +stopped resolving, the variable went out unset, and the server rejected the request with +`Unexpected unset value for bind variable 1`. This driver is more forgiving, but only on some of its +lookup paths — see below. The spelling can even vary from node to node during a rolling upgrade, +because the driver keeps whichever metadata the node that served the `PREPARE` sent back. + +The rule of thumb: **bind `?` markers positionally, and use named setters only for markers you named +yourself with `:name`.** + +If you address a synthesized name anyway, be aware of what the driver does and does not shield you +from: + +* the `String` setters match **case-insensitively** (see [AccessibleByName]), so + `setList("in(pk)", ...)` still finds a variable that the server called `IN(pk)`. A change of case + alone is survivable; +* the [CqlIdentifier] setters do **not** — they match exactly, so + `setList(CqlIdentifier.fromInternal("in(pk)"), ...)` does not resolve against a variable the + server called `IN(pk)`, and throws `IllegalArgumentException` rather than quietly leaving it + unset. Nor can you build the identifier from its CQL form: `CqlIdentifier.fromCql("IN(pk)")` + throws outright, because the parentheses would have to be double-quoted; +* a double-quoted `String` name such as `setList("\"in(pk)\"", ...)` also forces an exact match, + and throws in the same way. + +Note that the exception comes from the setter. Querying the metadata directly reports the same miss +without throwing: `getVariableDefinitions().firstIndexOf(...)` returns `-1`, and `allIndicesOf(...)` +returns an empty list. + +Finally, a named setter writes **every** variable that matches the name, not just the first one. +Names are therefore ambiguous whenever a query mentions the same column more than once, as in +`select * from sales where sku = ? and date > ? and date < ?` or `... where a in ? and a in ?`. Bind +those markers positionally, or name them apart yourself — `... and date > :from and date < :to`. #### Unset values @@ -234,6 +270,9 @@ bound = bound.unset("description"); bound = bound.unset(1); ``` +For brevity this example addresses `ps1`'s anonymous markers by their synthesized names; in +application code, prefer the positional form for the reasons given above. + A bound statement also has getters to retrieve the values. Note that this has a small performance overhead, since values are stored in their serialized form. @@ -350,6 +389,8 @@ new version with the response; the driver updates its local cache transparently, observe the new columns in the result set. [BoundStatement]: https://docs.datastax.com/en/drivers/java/4.17/com/datastax/oss/driver/api/core/cql/BoundStatement.html +[AccessibleByName]: https://docs.datastax.com/en/drivers/java/4.17/com/datastax/oss/driver/api/core/data/AccessibleByName.html +[CqlIdentifier]: https://docs.datastax.com/en/drivers/java/4.17/com/datastax/oss/driver/api/core/CqlIdentifier.html [Session.prepare]: https://docs.datastax.com/en/drivers/java/4.17/com/datastax/oss/driver/api/core/CqlSession.html#prepare-com.datastax.oss.driver.api.core.cql.SimpleStatement- [CASSANDRA-10786]: https://issues.apache.org/jira/browse/CASSANDRA-10786 [CASSANDRA-10813]: https://issues.apache.org/jira/browse/CASSANDRA-10813 diff --git a/manual/core/statements/simple/README.md b/manual/core/statements/simple/README.md index 13ddbb7a389..90a64c6d106 100644 --- a/manual/core/statements/simple/README.md +++ b/manual/core/statements/simple/README.md @@ -123,6 +123,16 @@ separately: .build(); ``` +Unlike a prepared statement, a simple statement is not parsed by the driver, so there is no local +variable metadata to match the names against: the driver sends them with the query, and the +**coordinator** resolves them. Name your markers yourself with `:name`, and fill anonymous `?` +markers positionally — the names the server synthesizes for anonymous markers (see [prepared +statements](../prepared/#anonymous-markers-and-server-synthesized-names)) are even less usable +here. The `String`-keyed methods do put each name through `CqlIdentifier.fromCql`, which +lower-cases it and rejects a parenthesised one outright; past that check the driver has nothing to +compare against, so a name that is merely wrong is only rejected by the coordinator, at execution +time. Mixing positional and named values in one statement is rejected outright. + This syntax has a few advantages: * if the values come from some other part of your code, it looks cleaner than doing the diff --git a/mapper-processor/src/main/java/com/datastax/oss/driver/internal/mapper/processor/entity/BuiltInNameConversions.java b/mapper-processor/src/main/java/com/datastax/oss/driver/internal/mapper/processor/entity/BuiltInNameConversions.java index 5c225fa8bf8..970a0c541eb 100644 --- a/mapper-processor/src/main/java/com/datastax/oss/driver/internal/mapper/processor/entity/BuiltInNameConversions.java +++ b/mapper-processor/src/main/java/com/datastax/oss/driver/internal/mapper/processor/entity/BuiltInNameConversions.java @@ -21,6 +21,7 @@ import com.datastax.oss.driver.api.mapper.entity.naming.NamingConvention; import com.datastax.oss.driver.internal.core.util.Strings; import com.datastax.oss.driver.shaded.guava.common.base.CaseFormat; +import java.util.Locale; /** * Handles the {@link NamingConvention built-in naming conventions}. @@ -49,7 +50,12 @@ public static String toCassandraName(String javaName, NamingConvention conventio return Strings.doubleQuote( CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_UNDERSCORE, javaName)); case UPPER_CASE: - return Strings.doubleQuote(javaName.toUpperCase()); + // ROOT, not the default locale: in a Turkish JVM a property named id would upper-case to a + // dotted capital I, and the generated code would reference a column that does not exist. + // This is the only branch that folds case itself: CASE_INSENSITIVE and EXACT_CASE fold + // nothing, and the camel and snake ones delegate to Guava's CaseFormat, which is ASCII-only + // and locale-neutral. + return Strings.doubleQuote(javaName.toUpperCase(Locale.ROOT)); default: throw new AssertionError("Unsupported convention: " + convention); } diff --git a/mapper-processor/src/test/java/com/datastax/oss/driver/internal/mapper/processor/entity/BuiltInNameConversionsTest.java b/mapper-processor/src/test/java/com/datastax/oss/driver/internal/mapper/processor/entity/BuiltInNameConversionsTest.java index cc5222c4f9a..80871fe777a 100644 --- a/mapper-processor/src/test/java/com/datastax/oss/driver/internal/mapper/processor/entity/BuiltInNameConversionsTest.java +++ b/mapper-processor/src/test/java/com/datastax/oss/driver/internal/mapper/processor/entity/BuiltInNameConversionsTest.java @@ -26,9 +26,15 @@ import static com.datastax.oss.driver.api.mapper.entity.naming.NamingConvention.UPPER_SNAKE_CASE; import static org.assertj.core.api.Assertions.assertThat; +import com.datastax.oss.driver.TestDataProviders; import com.datastax.oss.driver.api.mapper.entity.naming.NamingConvention; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.util.Locale; import org.junit.Test; +import org.junit.runner.RunWith; +@RunWith(DataProviderRunner.class) public class BuiltInNameConversionsTest { @Test @@ -55,6 +61,28 @@ public void should_convert_to_cql() { should_convert_to_cql("productId", UPPER_CASE, "\"PRODUCTID\""); } + /** + * UPPER_CASE folds the property name itself, so in a Turkish JVM a lower-case i becomes a dotted + * capital I and the generated code would reference a column that does not exist. The names below + * deliberately carry a lower-case i: "productId" would not do, since its I is already capital and + * no locale changes it. The UPPER_SNAKE_CASE case pins the claim that Guava's CaseFormat is + * genuinely locale-neutral rather than assumed to be; it covers the four camel- and snake-case + * conventions that delegate to it. CASE_INSENSITIVE and EXACT_CASE fold no case at all. + */ + @Test + @UseDataProvider(location = TestDataProviders.class, value = "locales") + public void should_convert_to_cql_in_any_default_locale(Locale locale) { + Locale def = Locale.getDefault(); + try { + Locale.setDefault(locale); + should_convert_to_cql("id", UPPER_CASE, "\"ID\""); + should_convert_to_cql("minPrice", UPPER_CASE, "\"MINPRICE\""); + should_convert_to_cql("minPrice", UPPER_SNAKE_CASE, "\"MIN_PRICE\""); + } finally { + Locale.setDefault(def); + } + } + private void should_convert_to_cql( String javaName, NamingConvention convention, String expectedCqlName) { String actualCqlName = BuiltInNameConversions.toCassandraName(javaName, convention);