You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "wirybeaver (via GitHub)" <gi...@apache.org> on 2024/01/08 17:21:59 UTC
[PR] skip invalid json string rather than throwing error during json indexing [pinot]
wirybeaver opened a new pull request, #12238:
URL: https://github.com/apache/pinot/pull/12238
Add a new configuration to skip broken json string and continue indexing following json strings instead of throw exception.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "chenboat (via GitHub)" <gi...@apache.org>.
chenboat commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1467981025
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +374,50 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJsonEnable() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
+ // the braces don't match and cannot be parsed
+ String[] records = {"{\"key1\":\"va\""};
+
+ createIndex(true, jsonIndexConfig, records);
+ File onHeapIndexFile = new File(INDEX_DIR, ON_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(onHeapIndexFile.exists());
+
+ createIndex(false, jsonIndexConfig, records);
+ File offHeapIndexFile = new File(INDEX_DIR, OFF_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(offHeapIndexFile.exists());
+
+ try (PinotDataBuffer onHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(onHeapIndexFile);
+ PinotDataBuffer offHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(offHeapIndexFile);
+ JsonIndexReader onHeapIndexReader = new ImmutableJsonIndexReader(onHeapDataBuffer, records.length);
+ JsonIndexReader offHeapIndexReader = new ImmutableJsonIndexReader(offHeapDataBuffer, records.length);
+ MutableJsonIndexImpl mutableJsonIndex = new MutableJsonIndexImpl(jsonIndexConfig)) {
+ for (String record : records) {
+ mutableJsonIndex.add(record);
+ }
+ Map<String, RoaringBitmap> onHeapRes = onHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> offHeapRes = offHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> mutableRes = mutableJsonIndex.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> expectedRes = Collections.singletonMap(JsonUtils.SKIPPED_VALUE_REPLACEMENT,
+ RoaringBitmap.bitmapOf(0));
+ Assert.assertEquals(expectedRes, onHeapRes);
+ Assert.assertEquals(expectedRes, offHeapRes);
+ Assert.assertEquals(expectedRes, mutableRes);
+ }
+ }
+
+ @Test(expectedExceptions = JsonProcessingException.class)
+ public void testSkipInvalidJsonDisable() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(false);
Review Comment:
Can we remove this to test the default behavior?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #12238:
URL: https://github.com/apache/pinot/pull/12238#issuecomment-1935085148
Merged since very minor changes since Rong's last review.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1468024267
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +374,50 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJsonEnable() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
+ // the braces don't match and cannot be parsed
+ String[] records = {"{\"key1\":\"va\""};
+
+ createIndex(true, jsonIndexConfig, records);
+ File onHeapIndexFile = new File(INDEX_DIR, ON_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(onHeapIndexFile.exists());
+
+ createIndex(false, jsonIndexConfig, records);
+ File offHeapIndexFile = new File(INDEX_DIR, OFF_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(offHeapIndexFile.exists());
+
+ try (PinotDataBuffer onHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(onHeapIndexFile);
+ PinotDataBuffer offHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(offHeapIndexFile);
+ JsonIndexReader onHeapIndexReader = new ImmutableJsonIndexReader(onHeapDataBuffer, records.length);
+ JsonIndexReader offHeapIndexReader = new ImmutableJsonIndexReader(offHeapDataBuffer, records.length);
+ MutableJsonIndexImpl mutableJsonIndex = new MutableJsonIndexImpl(jsonIndexConfig)) {
+ for (String record : records) {
+ mutableJsonIndex.add(record);
+ }
+ Map<String, RoaringBitmap> onHeapRes = onHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> offHeapRes = offHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> mutableRes = mutableJsonIndex.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> expectedRes = Collections.singletonMap(JsonUtils.SKIPPED_VALUE_REPLACEMENT,
+ RoaringBitmap.bitmapOf(0));
+ Assert.assertEquals(expectedRes, onHeapRes);
+ Assert.assertEquals(expectedRes, offHeapRes);
+ Assert.assertEquals(expectedRes, mutableRes);
+ }
+ }
+
+ @Test(expectedExceptions = JsonProcessingException.class)
+ public void testSkipInvalidJsonDisable() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(false);
Review Comment:
false is the default option in OSS for backward compatibility.
I will flip the default value in company's internal Pinot repo, thus don't want to remove this line.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1460134339
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -719,4 +719,18 @@ private static void addFieldToPinotSchema(Schema pinotSchema, DataType dataType,
}
}
}
+
+ public static List<Map<String, String>> flatten(String jsonString, JsonIndexConfig jsonIndexConfig) throws IOException {
+ JsonNode jsonNode;
+ try {
+ jsonNode = JsonUtils.stringToJsonNode(jsonString);
+ } catch (IOException e) {
Review Comment:
@ankitsultana @walterddr
I change the source catch JsonProcessingException in the most recent commit because the stringToJsonNode internal invoke the DEFAULT_READER.readTree() which throws JsonProcessingException only.
```
JsonUtils.stringToJsonNode(...) throws IOException {
DEFAULT_READER.readTree(...);
}
class ObjectReader {
public JsonNode readTree(String json) throws JsonProcessingException, JsonMappingException;
}
```
JsonMappingException is a child of JsonProcessingException.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1448175846
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -719,4 +719,18 @@ private static void addFieldToPinotSchema(Schema pinotSchema, DataType dataType,
}
}
}
+
+ public static List<Map<String, String>> flatten(String jsonString, JsonIndexConfig jsonIndexConfig) throws IOException {
+ JsonNode jsonNode;
+ try {
+ jsonNode = JsonUtils.stringToJsonNode(jsonString);
+ } catch (IOException e) {
+ if (jsonIndexConfig.getSkipInvalidJson()) {
+ return Collections.singletonList(Collections.singletonMap(VALUE_KEY, SKIPPED_VALUE_REPLACEMENT));
Review Comment:
why create this singleton at runtime? can't we make a static obj?
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -719,4 +719,18 @@ private static void addFieldToPinotSchema(Schema pinotSchema, DataType dataType,
}
}
}
+
+ public static List<Map<String, String>> flatten(String jsonString, JsonIndexConfig jsonIndexConfig) throws IOException {
+ JsonNode jsonNode;
+ try {
+ jsonNode = JsonUtils.stringToJsonNode(jsonString);
+ } catch (IOException e) {
Review Comment:
i think the actual exception here we supposed to cathc is malform json exception only. all other issues should still throw --> e..g we should not return incorrect result or simply replace that with the default when there are other issues.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1482237810
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -719,4 +721,19 @@ private static void addFieldToPinotSchema(Schema pinotSchema, DataType dataType,
}
}
}
+
+ public static List<Map<String, String>> flatten(String jsonString, JsonIndexConfig jsonIndexConfig)
Review Comment:
You are right. Make the method protected
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on PR #12238:
URL: https://github.com/apache/pinot/pull/12238#issuecomment-1933262709
@walterddr Could you review again to see if there's any objection?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1453889849
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +372,40 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJson() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
Review Comment:
sure
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1451904644
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -719,4 +719,18 @@ private static void addFieldToPinotSchema(Schema pinotSchema, DataType dataType,
}
}
}
+
+ public static List<Map<String, String>> flatten(String jsonString, JsonIndexConfig jsonIndexConfig) throws IOException {
+ JsonNode jsonNode;
+ try {
+ jsonNode = JsonUtils.stringToJsonNode(jsonString);
+ } catch (IOException e) {
Review Comment:
Thanks for the explanation @walterddr ,
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1482203477
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -719,4 +721,19 @@ private static void addFieldToPinotSchema(Schema pinotSchema, DataType dataType,
}
}
}
+
+ public static List<Map<String, String>> flatten(String jsonString, JsonIndexConfig jsonIndexConfig)
Review Comment:
@wirybeaver : what about the other `JsonUtils#flatten` methods? Are they still being used? I see them only being used in tests (on a different branch). If so let's make them protected.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana merged PR #12238:
URL: https://github.com/apache/pinot/pull/12238
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1460134339
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -719,4 +719,18 @@ private static void addFieldToPinotSchema(Schema pinotSchema, DataType dataType,
}
}
}
+
+ public static List<Map<String, String>> flatten(String jsonString, JsonIndexConfig jsonIndexConfig) throws IOException {
+ JsonNode jsonNode;
+ try {
+ jsonNode = JsonUtils.stringToJsonNode(jsonString);
+ } catch (IOException e) {
Review Comment:
@ankitsultana @walterddr
I change the source catch JsonProcessingException because the stringToJsonNode internal invoke the DEFAULT_READER.readTree() which throws JsonProcessingException only.
```
JsonUtils.stringToJsonNode(...) throws IOException {
DEFAULT_READER.readTree(...);
}
class ObjectReader {
public JsonNode readTree(String json) throws JsonProcessingException, JsonMappingException;
}
```
JsonMappingException is a child of JsonProcessingException.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1460136520
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +372,40 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJson() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
Review Comment:
> can you
>
> 1. add a test when it is false it still throws?
> 2. add a test for other exception is still throws when this is set to true?
for the 2nd requirement, I haven't figure a way to test since the JsonUtils make the objectReader as a static final member and I cannot replace a static final member without changing the constructor signature or not using more powerful mock libraray.
```
public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
```
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "chenboat (via GitHub)" <gi...@apache.org>.
chenboat commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1468183357
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +374,50 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJsonEnable() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
+ // the braces don't match and cannot be parsed
+ String[] records = {"{\"key1\":\"va\""};
+
+ createIndex(true, jsonIndexConfig, records);
+ File onHeapIndexFile = new File(INDEX_DIR, ON_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(onHeapIndexFile.exists());
+
+ createIndex(false, jsonIndexConfig, records);
+ File offHeapIndexFile = new File(INDEX_DIR, OFF_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(offHeapIndexFile.exists());
+
+ try (PinotDataBuffer onHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(onHeapIndexFile);
+ PinotDataBuffer offHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(offHeapIndexFile);
+ JsonIndexReader onHeapIndexReader = new ImmutableJsonIndexReader(onHeapDataBuffer, records.length);
+ JsonIndexReader offHeapIndexReader = new ImmutableJsonIndexReader(offHeapDataBuffer, records.length);
+ MutableJsonIndexImpl mutableJsonIndex = new MutableJsonIndexImpl(jsonIndexConfig)) {
+ for (String record : records) {
+ mutableJsonIndex.add(record);
+ }
+ Map<String, RoaringBitmap> onHeapRes = onHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> offHeapRes = offHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> mutableRes = mutableJsonIndex.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> expectedRes = Collections.singletonMap(JsonUtils.SKIPPED_VALUE_REPLACEMENT,
+ RoaringBitmap.bitmapOf(0));
+ Assert.assertEquals(expectedRes, onHeapRes);
+ Assert.assertEquals(expectedRes, offHeapRes);
+ Assert.assertEquals(expectedRes, mutableRes);
+ }
+ }
+
+ @Test(expectedExceptions = JsonProcessingException.class)
+ public void testSkipInvalidJsonDisable() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(false);
Review Comment:
My point is that for most users they do not need to explicitly set the flag because the default behavior is false. I think the test should make sure such users will not be affected by this PR.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1453881420
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +372,40 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJson() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
Review Comment:
can you
1. add a test when it is false it still throws?
2. add a test for other exception is still throws when this is set to true?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1450677667
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -719,4 +719,18 @@ private static void addFieldToPinotSchema(Schema pinotSchema, DataType dataType,
}
}
}
+
+ public static List<Map<String, String>> flatten(String jsonString, JsonIndexConfig jsonIndexConfig) throws IOException {
+ JsonNode jsonNode;
+ try {
+ jsonNode = JsonUtils.stringToJsonNode(jsonString);
+ } catch (IOException e) {
+ if (jsonIndexConfig.getSkipInvalidJson()) {
+ return Collections.singletonList(Collections.singletonMap(VALUE_KEY, SKIPPED_VALUE_REPLACEMENT));
Review Comment:
good catch
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on PR #12238:
URL: https://github.com/apache/pinot/pull/12238#issuecomment-1933117409
> Apologies. Had stamped but had a question. @wirybeaver can you take a look?
no worries
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "chenboat (via GitHub)" <gi...@apache.org>.
chenboat commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1467980253
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +374,50 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJsonEnable() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
+ // the braces don't match and cannot be parsed
+ String[] records = {"{\"key1\":\"va\""};
+
+ createIndex(true, jsonIndexConfig, records);
+ File onHeapIndexFile = new File(INDEX_DIR, ON_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(onHeapIndexFile.exists());
+
+ createIndex(false, jsonIndexConfig, records);
+ File offHeapIndexFile = new File(INDEX_DIR, OFF_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(offHeapIndexFile.exists());
+
+ try (PinotDataBuffer onHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(onHeapIndexFile);
+ PinotDataBuffer offHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(offHeapIndexFile);
+ JsonIndexReader onHeapIndexReader = new ImmutableJsonIndexReader(onHeapDataBuffer, records.length);
+ JsonIndexReader offHeapIndexReader = new ImmutableJsonIndexReader(offHeapDataBuffer, records.length);
+ MutableJsonIndexImpl mutableJsonIndex = new MutableJsonIndexImpl(jsonIndexConfig)) {
+ for (String record : records) {
+ mutableJsonIndex.add(record);
+ }
+ Map<String, RoaringBitmap> onHeapRes = onHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> offHeapRes = offHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> mutableRes = mutableJsonIndex.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> expectedRes = Collections.singletonMap(JsonUtils.SKIPPED_VALUE_REPLACEMENT,
+ RoaringBitmap.bitmapOf(0));
+ Assert.assertEquals(expectedRes, onHeapRes);
+ Assert.assertEquals(expectedRes, offHeapRes);
+ Assert.assertEquals(expectedRes, mutableRes);
+ }
+ }
+
+ @Test(expectedExceptions = JsonProcessingException.class)
+ public void testSkipInvalidJsonDisable() throws Exception {
Review Comment:
nit: Disable --> Disabled
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1468198505
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +374,50 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJsonEnable() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
+ // the braces don't match and cannot be parsed
+ String[] records = {"{\"key1\":\"va\""};
+
+ createIndex(true, jsonIndexConfig, records);
+ File onHeapIndexFile = new File(INDEX_DIR, ON_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(onHeapIndexFile.exists());
+
+ createIndex(false, jsonIndexConfig, records);
+ File offHeapIndexFile = new File(INDEX_DIR, OFF_HEAP_COLUMN_NAME + V1Constants.Indexes.JSON_INDEX_FILE_EXTENSION);
+ Assert.assertTrue(offHeapIndexFile.exists());
+
+ try (PinotDataBuffer onHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(onHeapIndexFile);
+ PinotDataBuffer offHeapDataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(offHeapIndexFile);
+ JsonIndexReader onHeapIndexReader = new ImmutableJsonIndexReader(onHeapDataBuffer, records.length);
+ JsonIndexReader offHeapIndexReader = new ImmutableJsonIndexReader(offHeapDataBuffer, records.length);
+ MutableJsonIndexImpl mutableJsonIndex = new MutableJsonIndexImpl(jsonIndexConfig)) {
+ for (String record : records) {
+ mutableJsonIndex.add(record);
+ }
+ Map<String, RoaringBitmap> onHeapRes = onHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> offHeapRes = offHeapIndexReader.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> mutableRes = mutableJsonIndex.getMatchingDocsMap("");
+ Map<String, RoaringBitmap> expectedRes = Collections.singletonMap(JsonUtils.SKIPPED_VALUE_REPLACEMENT,
+ RoaringBitmap.bitmapOf(0));
+ Assert.assertEquals(expectedRes, onHeapRes);
+ Assert.assertEquals(expectedRes, offHeapRes);
+ Assert.assertEquals(expectedRes, mutableRes);
+ }
+ }
+
+ @Test(expectedExceptions = JsonProcessingException.class)
+ public void testSkipInvalidJsonDisable() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(false);
Review Comment:
OK, that means I need to not only flip the default value but also add `jsonIndexConfig.setSkipInvalidJson(false)` in the company's internal follow up patch.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1447978602
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -719,4 +719,18 @@ private static void addFieldToPinotSchema(Schema pinotSchema, DataType dataType,
}
}
}
+
+ public static List<Map<String, String>> flatten(String jsonString, JsonIndexConfig jsonIndexConfig) throws IOException {
+ JsonNode jsonNode;
+ try {
+ jsonNode = JsonUtils.stringToJsonNode(jsonString);
+ } catch (IOException e) {
Review Comment:
Why limit to IOException? Can we catch a `Throwable` here?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12238:
URL: https://github.com/apache/pinot/pull/12238#issuecomment-1881585866
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `15 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`b9ed378`)](https://app.codecov.io/gh/apache/pinot/commit/b9ed378355acc5313ebc1031510671f7045f29a9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.65% compared to head [(`a60cf46`)](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 0.00%.
> Report is 82 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ain/java/org/apache/pinot/spi/utils/JsonUtils.java](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvSnNvblV0aWxzLmphdmE=) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...apache/pinot/spi/config/table/JsonIndexConfig.java](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0pzb25JbmRleENvbmZpZy5qYXZh) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...local/realtime/impl/json/MutableJsonIndexImpl.java](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2pzb24vTXV0YWJsZUpzb25JbmRleEltcGwuamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...nt/creator/impl/inv/json/BaseJsonIndexCreator.java](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9pbnYvanNvbi9CYXNlSnNvbkluZGV4Q3JlYXRvci5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12238 +/- ##
=============================================
- Coverage 61.65% 0.00% -61.65%
+ Complexity 1152 6 -1146
=============================================
Files 2389 2340 -49
Lines 129819 127444 -2375
Branches 20082 19689 -393
=============================================
- Hits 80036 6 -80030
- Misses 43958 127438 +83480
+ Partials 5825 0 -5825
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.53%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.61%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.49%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.65%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12238/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12238?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1460136520
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +372,40 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJson() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
Review Comment:
> can you
>
> 1. add a test when it is false it still throws?
> 2. add a test for other exception is still throws when this is set to true?
for the 2nd requirement, I haven't figure a way to test since the JsonUtils make the objectReader as a static final member.
```
public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
```
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12238:
URL: https://github.com/apache/pinot/pull/12238#discussion_r1465669622
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java:
##########
@@ -371,6 +372,40 @@ public void testGetValuesForKeyAndDocs()
}
}
+ @Test
+ public void testSkipInvalidJson() throws Exception {
+ JsonIndexConfig jsonIndexConfig = new JsonIndexConfig();
+ jsonIndexConfig.setSkipInvalidJson(true);
Review Comment:
sorry for the late reply. that fine if we can't find it.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] skip invalid json string rather than throwing error during json indexing [pinot]
Posted by "wirybeaver (via GitHub)" <gi...@apache.org>.
wirybeaver commented on PR #12238:
URL: https://github.com/apache/pinot/pull/12238#issuecomment-1908900521
Deployed this change to the company's prod cluster with following config and the ERROR segments become ONLINE.
```
"fieldConfigList": [
{
"name": "foo",
"encodingType": "RAW",
"indexTypes": [],
"compressionCodec": "ZSTANDARD",
"indexes": {
"json": {
"maxLevels": 2,
"disableCrossArrayUnnest": true,
"skipInvalidJson": true
},
"text": {}
},
"properties": {
"deriveNumDocsPerChunkForRawIndex": "true",
"rawIndexWriterVersion": "3",
},
"tierOverwrites": null
}
]
```
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org