This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new ef7b1b7a733 perf: avoid materializing skipped Fory JSON strings
(#19672)
ef7b1b7a733 is described below
commit ef7b1b7a733bd85d7d49bf46bd3a4d5e7217d3ab
Author: Shawn Yang <[email protected]>
AuthorDate: Sun Sep 27 08:33:49 2026 +0800
perf: avoid materializing skipped Fory JSON strings (#19672)
---
.../common/function/ForyJsonPathExtractor.java | 15 +++++----
.../common/function/scalar/JsonFunctions.java | 6 ++--
.../function/ForyJsonLinkageFallbackTest.java | 37 ++++++++++++++++++++++
.../common/function/ForyJsonPathFunctionsTest.java | 27 ++++++++++++++++
.../JsonExtractScalarTransformFunction.java | 4 +--
5 files changed, 78 insertions(+), 11 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/function/ForyJsonPathExtractor.java
b/pinot-common/src/main/java/org/apache/pinot/common/function/ForyJsonPathExtractor.java
index bfe32acc725..8919344a648 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/function/ForyJsonPathExtractor.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/function/ForyJsonPathExtractor.java
@@ -38,7 +38,7 @@ import org.slf4j.LoggerFactory;
/// Streaming extraction uses one bounded parser pool shared by all worker
threads. Initialization or runtime linkage
/// failures permanently disable the optional path, allowing callers to fall
back to Jackson/Jayway. Jackson's
/// document, token, nesting, field-name, string, and number constraints are
enforced while walking the document.
-/// The Fory runtime is an optional dependency; applications must add
`org.apache.fory:fory-json:1.6.0` (and its
+/// The Fory runtime is an optional dependency; applications must add
`org.apache.fory:fory-json:1.7.5` (and its
/// transitive `fory-core` dependency) to the application classpath to enable
this experimental path.
public final class ForyJsonPathExtractor {
private static final Logger LOGGER =
LoggerFactory.getLogger(ForyJsonPathExtractor.class);
@@ -107,6 +107,7 @@ public final class ForyJsonPathExtractor {
context._path = path;
context._result = null;
context._tokenCount = 0;
+ context._checkStringLength = json.length() >
JACKSON_CONSTRAINTS.getMaxStringLength();
try {
parser.fromJson(json, PathResult.class);
return context._result;
@@ -118,6 +119,7 @@ public final class ForyJsonPathExtractor {
context._path = null;
context._result = null;
context._tokenCount = 0;
+ context._checkStringLength = false;
context._active = false;
}
}
@@ -262,11 +264,11 @@ public final class ForyJsonPathExtractor {
}
if (token == '"') {
countToken(context);
- // Fory 1.6's skipValue() computes an FNV hash over every character. Its
string decoder uses packed scans and
- // is substantially faster even when the decoded value is discarded. An
upstream fast-skip API could remove
- // this temporary allocation in a future Fory version.
- String value = reader.readString();
- if (value.length() > JACKSON_CONSTRAINTS.getMaxStringLength()) {
+ // A decoded string cannot exceed its containing String document. Larger
documents need decoded lengths:
+ // raw token lengths overcount escapes. The reader's reusable text view
avoids materializing a String.
+ if (!context._checkStringLength) {
+ reader.skipValue();
+ } else if (reader.readQuotedText().length() >
JACKSON_CONSTRAINTS.getMaxStringLength()) {
throw new IllegalArgumentException("JSON string exceeds Jackson's
configured limit");
}
return;
@@ -330,6 +332,7 @@ public final class ForyJsonPathExtractor {
private static final class PathContext {
private final PathResult _marker = new PathResult();
private boolean _active;
+ private boolean _checkStringLength;
private long _tokenCount;
@Nullable
private SimpleJsonPath _path;
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
index c8649a8add1..423755c29ed 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
@@ -305,7 +305,7 @@ public class JsonFunctions {
/// **Experimental.** Fory-backed variant of [#jsonPathString(Object,
String, String)]. Fory parses JSON strings
/// for simple paths; unsupported inputs and parse failures fall back to
Jayway. This function can change or be
/// removed while the Fory integration is evaluated and must be explicitly
selected by name. The optional
- /// `org.apache.fory:fory-json:1.6.0` runtime must be on the application
classpath to activate Fory; otherwise this
+ /// `org.apache.fory:fory-json:1.7.5` runtime must be on the application
classpath to activate Fory; otherwise this
/// function uses Jayway.
@ScalarFunction(nullableParameters = true)
public static String jsonPathStringFory(@Nullable Object object, String
jsonPath, String defaultValue) {
@@ -396,7 +396,7 @@ public class JsonFunctions {
/// **Experimental.** Fory-backed variant of [#jsonPathLong(Object, String,
long)]. Unsupported inputs and parse
/// failures fall back to Jayway. This function can change or be removed
while the Fory integration is evaluated.
- /// The optional `org.apache.fory:fory-json:1.6.0` runtime must be on the
application classpath to activate Fory.
+ /// The optional `org.apache.fory:fory-json:1.7.5` runtime must be on the
application classpath to activate Fory.
@ScalarFunction(nullableParameters = true)
public static long jsonPathLongFory(@Nullable Object object, String
jsonPath, long defaultValue) {
if (!canExtractJsonPath(object)) {
@@ -485,7 +485,7 @@ public class JsonFunctions {
/// **Experimental.** Fory-backed variant of [#jsonPathDouble(Object,
String, double)]. Unsupported inputs and
/// parse failures fall back to Jayway. This function can change or be
removed while the Fory integration is
- /// evaluated. The optional `org.apache.fory:fory-json:1.6.0` runtime must
be on the application classpath to
+ /// evaluated. The optional `org.apache.fory:fory-json:1.7.5` runtime must
be on the application classpath to
/// activate Fory.
@ScalarFunction(nullableParameters = true)
public static double jsonPathDoubleFory(@Nullable Object object, String
jsonPath, double defaultValue) {
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/function/ForyJsonLinkageFallbackTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/function/ForyJsonLinkageFallbackTest.java
index 2169d553724..2a0928a7734 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/function/ForyJsonLinkageFallbackTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/function/ForyJsonLinkageFallbackTest.java
@@ -29,6 +29,7 @@ import org.apache.pinot.common.function.scalar.JsonFunctions;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertThrows;
import static org.testng.Assert.assertTrue;
@@ -36,6 +37,7 @@ import static org.testng.Assert.assertTrue;
public class ForyJsonLinkageFallbackTest {
private static final String FALLBACK_ARGUMENT = "verifyFallback";
private static final String TOKEN_LIMIT_ARGUMENT = "verifyTokenLimit";
+ private static final String STRING_LIMIT_ARGUMENT = "verifyStringLimit";
/// Child-process entry point used by the missing-Fory fallback tests.
public static void main(String[] arguments) {
@@ -46,6 +48,10 @@ public class ForyJsonLinkageFallbackTest {
verifyConfiguredTokenLimit();
return;
}
+ if (STRING_LIMIT_ARGUMENT.equals(arguments[0])) {
+ verifyConfiguredStringLimit();
+ return;
+ }
if (!FALLBACK_ARGUMENT.equals(arguments[0])) {
throw new IllegalArgumentException("Unknown child-process verification
argument: " + arguments[0]);
}
@@ -81,6 +87,37 @@ public class ForyJsonLinkageFallbackTest {
runChild(false, false, TOKEN_LIMIT_ARGUMENT);
}
+ @Test
+ public void testConfiguredJacksonStringLimit()
+ throws Exception {
+ runChild(false, false, STRING_LIMIT_ARGUMENT);
+ }
+
+ private static void verifyConfiguredStringLimit() {
+ int maxLength = 64;
+ StreamReadConstraints.overrideDefaultStreamReadConstraints(
+ StreamReadConstraints.builder().maxStringLength(maxLength).build());
+ SimpleJsonPath path = SimpleJsonPath.compile("$.v");
+ assertTrue(path != null);
+ assertTrue(ForyJsonPathExtractor.isAvailable());
+ // Escaped text may exceed the raw length limit while its decoded UTF-16
length is still valid.
+ for (String unit : new String[]{"x", "é", "中", "\\u0061", "\\n"}) {
+ String value = unit.repeat(maxLength);
+ assertEquals(ForyJsonPathExtractor.extract("{\"ignored\":\"" + value +
"\",\"v\":7}", path), 7L);
+ assertThrows(IllegalArgumentException.class,
+ () -> ForyJsonPathExtractor.extract("{\"v\":7,\"ignored\":\"" +
value + "x\"}", path));
+
assertEquals(ForyJsonPathExtractor.extract("{\"ignored\":\"ok\",\"v\":8}",
path), 8L);
+ }
+ for (String unit : new String[]{"😀", "\\uD83D\\uDE00"}) {
+ String value = unit.repeat(maxLength / 2);
+ assertEquals(ForyJsonPathExtractor.extract("{\"ignored\":[\"" + value +
"\"],\"v\":7}", path), 7L);
+ assertThrows(IllegalArgumentException.class,
+ () -> ForyJsonPathExtractor.extract("{\"ignored\":[\"" + value +
"x\"],\"v\":7}", path));
+ }
+ String fields = "\"ignored\":\"small\",".repeat(maxLength);
+ assertEquals(ForyJsonPathExtractor.extract("{" + fields + "\"v\":7}",
path), 7L);
+ }
+
private static void verifyConfiguredTokenLimit() {
StreamReadConstraints constraints =
StreamReadConstraints.builder().maxTokenCount(3).build();
StreamReadConstraints.overrideDefaultStreamReadConstraints(constraints);
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/function/ForyJsonPathFunctionsTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/function/ForyJsonPathFunctionsTest.java
index b57aadf7b9c..36593d236dc 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/function/ForyJsonPathFunctionsTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/function/ForyJsonPathFunctionsTest.java
@@ -148,6 +148,33 @@ public class ForyJsonPathFunctionsTest {
assertNull(extract("{\"v\":1,\"v\":null}", "$.v"));
}
+ @DataProvider(name = "skippedStrings")
+ public Object[][] skippedStrings() {
+ return new Object[][]{
+ {"x".repeat(65536)},
+ {"café".repeat(4096)},
+ {"中文😀".repeat(4096)},
+ {"\\u0061\\n\\\"\\\\\\uD83D\\uDE00".repeat(4096)}
+ };
+ }
+
+ @Test(dataProvider = "skippedStrings")
+ public void testStreamingSkippedStrings(String value) {
+ assertEquals(extract("{\"ignored\":\"" + value + "\",\"v\":7}", "$.v"),
7L);
+ assertEquals(extract("{\"v\":1,\"ignored\":{\"items\":[\"" + value +
"\"]},\"v\":7}", "$.v"), 7L);
+ assertEquals(extract("[\"" + value + "\",7,\"" + value + "\"]", "$[1]"),
7L);
+ }
+
+ @Test
+ public void testStreamingRejectsMalformedSkippedStrings() {
+ String prefix = "x".repeat(64);
+ for (String suffix : List.of("\\q", "\\uD800", "\\uDC00", "\\u123",
"\u0001")) {
+ String json = "{\"v\":7,\"ignored\":\"" + prefix + suffix + "\"}";
+ assertThrows(RuntimeException.class, () -> extract(json, "$.v"));
+ assertEquals(extract("{\"ignored\":\"ok\",\"v\":8}", "$.v"), 8L);
+ }
+ }
+
@Test
public void testStreamingExtractorReturnsNullForUnresolvedPaths() {
assertNull(extract("{\"a\":{\"v\":1}}", "$.missing"));
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java
index 78b054cb470..3ab114deed0 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java
@@ -79,7 +79,7 @@ import org.roaringbitmap.RoaringBitmap;
/// `STRING` input for scalar result types other than `STRING`, `JSON`, and
`BIG_DECIMAL`. `BYTES` input, complex
/// paths, containers / array result types, precision-sensitive results,
documents beyond Jackson's configured
/// limits, and Fory failures use Jayway. Its name, supported envelope, and
implementation can change while the
-/// integration is evaluated. Fory is an optional runtime dependency; add
`org.apache.fory:fory-json:1.6.0` (and its
+/// integration is evaluated. Fory is an optional runtime dependency; add
`org.apache.fory:fory-json:1.7.5` (and its
/// transitive `fory-core` dependency) to the application classpath to
activate this experimental path.
///
/// **Arguments:**
@@ -262,7 +262,7 @@ public class JsonExtractScalarTransformFunction extends
BaseTransformFunction {
_resultMetadata = new TransformResultMetadata(_dataType, isSingleValue,
false);
DataType inputDataType = firstArgument.getResultMetadata().getDataType();
// STRING/JSON use the BigDecimal-preserving parser because non-string
JSON values are serialized to the output.
- // Fory 1.6 materializes fractional numbers as Double, so enabling it here
would lose numeric precision.
+ // Fory materializes fractional numbers as Double, so enabling it here
would lose numeric precision.
_foryEligible = _extractionMode == ExtractionMode.FORY && _simpleJsonPath
!= null
&& inputDataType == DataType.STRING && isSingleValue && _dataType !=
DataType.STRING
&& _dataType != DataType.JSON && _dataType != DataType.BIG_DECIMAL;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
