Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -559,29 +559,68 @@ synchronized void update(Group groupIn) {
tablets = newTablets;
}

synchronized ChannelEndpoint fillRoutingHint(
Copy link
Contributor Author

@rahul2393 rahul2393 Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ChannelEndpoint fillRoutingHint(
boolean preferLeader,
DirectedReadOptions directedReadOptions,
RoutingHint.Builder hintBuilder) {
boolean hasDirectedReadOptions =
directedReadOptions.getReplicasCase()
!= DirectedReadOptions.ReplicasCase.REPLICAS_NOT_SET;

// Fast path: pick a tablet while holding the lock. If the endpoint is already
// cached on the tablet, return it immediately without releasing the lock.
// If the endpoint needs to be created (blocking network dial), release the
// lock first so other threads are not blocked during channel creation.
CachedTablet selected;
synchronized (this) {
selected =
selectTabletLocked(
preferLeader, hasDirectedReadOptions, hintBuilder, directedReadOptions);
if (selected == null) {
return null;
}
if (selected.endpoint != null || selected.serverAddress.isEmpty()) {
return selected.pick(hintBuilder);
}
// Slow path: endpoint not yet created. Capture the address and release the
// lock before calling endpointCache.get(), which may block on network dial.
hintBuilder.setTabletUid(selected.tabletUid);
}

String serverAddress = selected.serverAddress;
ChannelEndpoint endpoint = endpointCache.get(serverAddress);

synchronized (this) {
// Only update if the tablet's address hasn't changed since we released the lock.
if (selected.endpoint == null && selected.serverAddress.equals(serverAddress)) {
selected.endpoint = endpoint;
}
// Re-set tabletUid with the latest value in case update() ran concurrently.
hintBuilder.setTabletUid(selected.tabletUid);
return selected.endpoint;
}
}

private CachedTablet selectTabletLocked(
boolean preferLeader,
boolean hasDirectedReadOptions,
RoutingHint.Builder hintBuilder,
DirectedReadOptions directedReadOptions) {
if (preferLeader
&& !hasDirectedReadOptions
&& hasLeader()
&& leader().distance <= MAX_LOCAL_REPLICA_DISTANCE
&& !leader().shouldSkip(hintBuilder)) {
return leader().pick(hintBuilder);
return leader();
}

for (CachedTablet tablet : tablets) {
if (!tablet.matches(directedReadOptions)) {
continue;
}
if (tablet.shouldSkip(hintBuilder)) {
continue;
}
return tablet.pick(hintBuilder);
return tablet;
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@
import java.time.format.ResolverStyle;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ThreadLocalRandom;
import java.util.function.BiFunction;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -780,10 +784,26 @@ public TargetRange keySetToTargetRange(KeySet in) {
}

public TargetRange queryParamsToTargetRange(Struct in) {
// toLowerCase(Locale.ROOT) is safe for query parameter names, even for non-ASCII
// characters such as the Turkish upper-case İ (U+0130). Query parameter names cannot
// be quoted in Spanner SQL (the @paramName syntax imposes an unquoted identifier
// grammar), so both the identifier sent by the server in the KeyRecipe and the
// parameter name bound by the user must follow the same syntax rules. Applying the
// same Locale.ROOT case-folding to both sides guarantees a consistent match.
// If the server were to normalize identifiers differently, the only consequence is
// a routing miss and graceful fallback to the default endpoint — not a query failure.
//
// Sort field names before inserting into the map so that when two param names
// collide after case-folding (e.g. "Id" vs "ID") the winner is deterministic,
// matching the Go implementation.
List<String> fieldNames = new ArrayList<>(in.getFieldsMap().keySet());
Collections.sort(fieldNames);
final Map<String, Value> lowercaseFields = new HashMap<>(fieldNames.size());
for (String fieldName : fieldNames) {
lowercaseFields.put(fieldName.toLowerCase(Locale.ROOT), in.getFieldsMap().get(fieldName));
}
return encodeKeyInternal(
(index, identifier) -> {
return in.getFieldsMap().get(identifier);
},
(index, identifier) -> lowercaseFields.get(identifier.toLowerCase(Locale.ROOT)),
KeyType.FULL_KEY);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.cloud.spanner.spi.v1;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import com.google.protobuf.ByteString;
Expand Down Expand Up @@ -76,6 +77,87 @@ public void queryParamsUsesConstantValue() throws Exception {
assertTrue(target.limit.isEmpty());
}

@Test
public void queryParamsCaseInsensitiveFallback() throws Exception {
com.google.spanner.v1.KeyRecipe recipeProto =
createRecipe(
"part { tag: 1 }\n"
+ "part {\n"
+ " order: ASCENDING\n"
+ " null_order: NULLS_FIRST\n"
+ " type { code: STRING }\n"
+ " identifier: \"id\"\n"
+ "}\n");

Struct params =
parseStruct(
"fields {\n" + " key: \"Id\"\n" + " value { string_value: \"foo\" }\n" + "}\n");

KeyRecipe recipe = KeyRecipe.create(recipeProto);
TargetRange target = recipe.queryParamsToTargetRange(params);
assertEquals(expectedKey("foo"), target.start);
assertTrue(target.limit.isEmpty());
}

@Test
public void queryParamsCaseInsensitiveDuplicateUsesLastValue() throws Exception {
com.google.spanner.v1.KeyRecipe recipeProto =
createRecipe(
"part { tag: 1 }\n"
+ "part {\n"
+ " order: ASCENDING\n"
+ " null_order: NULLS_FIRST\n"
+ " type { code: STRING }\n"
+ " identifier: \"ID\"\n"
+ "}\n");

// Both "Id" and "id" normalize to "id"; the last one ("id"→"bar") wins.
Struct params =
parseStruct(
"fields {\n"
+ " key: \"Id\"\n"
+ " value { string_value: \"foo\" }\n"
+ "}\n"
+ "fields {\n"
+ " key: \"id\"\n"
+ " value { string_value: \"bar\" }\n"
+ "}\n");

KeyRecipe recipe = KeyRecipe.create(recipeProto);
TargetRange target = recipe.queryParamsToTargetRange(params);
assertEquals(expectedKey("bar"), target.start);
assertFalse(target.approximate);
assertTrue(target.limit.isEmpty());
}

@Test
public void queryParamsCaseInsensitiveSafeForTurkishDotI() throws Exception {
// Turkish upper-case İ (U+0130) lower-cases to two characters under Locale.ROOT
// (i + combining dot above), so "SİCİL".length() != "SİCİL".toLowerCase(ROOT).length()
// and "SİCİL".equalsIgnoreCase("SİCİL".toLowerCase(ROOT)) is false.
// This is still safe because both the recipe identifier (server-sent) and the user's
// bound parameter name go through the same Locale.ROOT lower-casing before the
// HashMap lookup, so they produce the same string on both sides and the match succeeds.
com.google.spanner.v1.KeyRecipe recipeProto =
createRecipe(
"part { tag: 1 }\n"
+ "part {\n"
+ " order: ASCENDING\n"
+ " null_order: NULLS_FIRST\n"
+ " type { code: STRING }\n"
+ " identifier: \"SİCİL\"\n"
+ "}\n");

Struct params =
parseStruct(
"fields {\n" + " key: \"SİCİL\"\n" + " value { string_value: \"test\" }\n" + "}\n");

KeyRecipe recipe = KeyRecipe.create(recipeProto);
TargetRange target = recipe.queryParamsToTargetRange(params);
assertEquals(expectedKey("test"), target.start);
assertTrue(target.limit.isEmpty());
}

private static com.google.spanner.v1.KeyRecipe createRecipe(String text)
throws TextFormat.ParseException {
com.google.spanner.v1.KeyRecipe.Builder builder = com.google.spanner.v1.KeyRecipe.newBuilder();
Expand Down
Loading