🟢 Observation: createPkSelectors refactoring changes behavior for nested PK paths in existing APIs

The old private createPkSelectors used StringUtils.substring(pathPart, 1) which treated /address/city as a single segment producing ["address/city"]. The new implementation uses PathParser.getPathParts which correctly splits into ["address"]["city"].

This is a bug fix for nested partition key paths (the old selector looked for a property literally named address/city rather than traversing nested objects), but it changes behavior for two existing code paths:

1. getRangeQueryMap → used by readMany(List<CosmosItemIdentity>)
2. createLogicalPartitionScanQuerySpec → used by readAllItemsOfLogicalPartition

The fix is correct and the risk is low (nested PK paths are uncommon, and the old behavior was wrong), but it may be worth noting in the CHANGELOG since it changes existing readMany behavior.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟡 Remaining: SELECT TOP N queries not rejected by validation

OFFSET and LIMIT were added to the rejection list (fixing the earlier comment), but queryInfo.hasTop() is still missing. SELECT TOP 5 * FROM c would pass validation and the SDK would split it across N physical partitions × M batches, each independently limiting to 5 rows — returning up to 5 × N × M results instead of the expected 5.

hasTop() is available on QueryInfo (line 71 of QueryInfo.java) and is used elsewhere in the codebase (e.g., DocumentQueryExecutionContextFactory:294).

Suggested fix: Add before the hasNonStreamingOrderBy() check:

if (queryInfo.hasTop()) {
    throw new IllegalArgumentException(
        "Custom query for readMany by partition key must not contain TOP.");
}

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🔴 Bug: Incorrect quote escaping

.map(pathPart -> "[""" + pathPart.replace("""", "\\") + """]")

.replace("""", "\\") replaces a double-quote with a single backslash, not an escaped quote \"". The intent is to produce valid JSON-style selectors like [""myField""].

Impact: If a partition key path part contains a " character, the generated selector is malformed the query will error or silently match the wrong field.

Fix:

.map(pathPart -> "[""" + pathPart.replace("""", "\\""") + """]")

(Note: the old createPkSelectors in RxDocumentClientImpl had the identical bug this is a pre-existing issue carried forward.)

⚠️ AI-generated review may be incorrect. Agree? resolve the conversation. Disagree? reply with your reasoning.

🟡 Design: get() and getCurrentRow() both advance the underlying iterator

override def get(): InternalRow = {
  cosmosRowConverter.fromRowToInternalRow(getOrCreateIterator.next().row, rowSerializer)
}

def getCurrentRow(): Row = getOrCreateIterator.next().row

Both methods call getOrCreateIterator.next(), which advances the iterator. Neither returns the current row both consume the next one.

Risk: If any code path calls both get() and getCurrentRow() for the same logical row, one item will be silently skipped. The naming getCurrentRow strongly implies a non-advancing read of the current position, but it actually advances. Even if current callers use them in separate contexts, the misleading API surface is a latent bug.

Suggestion: Consider caching the last next() result and having getCurrentRow() return the cached value without advancing, or renaming it to make the advancing behavior explicit (e.g., nextRow()).

⚠️ AI-generated review may be incorrect. Agree? resolve the conversation. Disagree? reply with your reasoning.