Re: [PR] [NoSQL] Count decoded length in IndexKey.skip [polaris]

["dimas-b (via GitHub)" <gi...@apache.org>]

dimas-b commented on code in PR #5636:
URL: https://github.com/apache/polaris/pull/5636#discussion_r4125779175


##########
persistence/nosql/persistence/api/src/test/java/org/apache/polaris/persistence/nosql/api/index/TestIndexKey.java:
##########
@@ -80,6 +82,41 @@ static Stream<String> keyTooLong() {
         STRING_100 + STRING_100 + STRING_100 + STRING_100 + STRING_100 + STRING_100);
   }
 
+  /**
+   * Keys whose decoded length is under {@link IndexKey#MAX_LENGTH} but that expand when escaped
+   * must still be skippable. {@code skip} used to count two units per escaped byte and reject these
+   * keys after they had already been written.
+   */
+  @ParameterizedTest
+  @ValueSource(chars = {'\u0001', '\u0002'})
+  void skipAcceptsKeysThatExpandWhenEscaped(char escapedChar) {
+    // Decoded length 251: under MAX_LENGTH, but 251 escape pairs used to push skip's counter past
+    // 500 (2 * 251) and throw while deserializeKey still succeeded.
+    var indexKey = key(String.valueOf(escapedChar).repeat(251));
+    var buffer = ByteBuffer.allocate(indexKey.serializedSize());
+    indexKey.serialize(buffer).flip();
+
+    assertThat(deserializeKey(buffer.duplicate())).isEqualTo(indexKey);
+    var skipped = buffer.duplicate();
+    skip(skipped);
+    assertThat(skipped.remaining()).isZero();
+  }
+
+  @Test
+  void skipAcceptsMixedEscapedKeyNearLimit() {
+    // 250 escaped bytes + one ordinary byte: decoded length 251. The old skip counter would reach
+    // 501 (2 * 250 + 1) and throw.
+    var raw = "\u0001".repeat(250) + "a";

Review Comment:
   This yields only 251 chars, which is not "near" the limit (according to the method name) 🤔 Should we go all the way to `MAX_LENGTH`?



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org