You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/12/07 17:10:00 UTC

[GitHub] [pinot] richardstartin opened a new pull request #7876: JSON indexing adapter

richardstartin opened a new pull request #7876:
URL: https://github.com/apache/pinot/pull/7876


   This makes the process of indexing JSON documents in a forward index pluggable, so the forward index doesn't need to store the documents as strings.


-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765947811



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexCreator.java
##########
@@ -29,7 +29,39 @@
   char KEY_VALUE_SEPARATOR = '\0';
 
   /**
-   * Adds the next json value.
+   * To be invoked once before each physical JSON document when streaming JSON into the index
+   * @param numFlattenedDocs the number of flattened documents to be streamed into the index
+   */
+  default void startDocument(int numFlattenedDocs) {
+  }
+
+  /**
+   * To be invoked after each JSON document when streaming JSON into the index.
+   */
+  default void endDocument() {
+  }
+
+  /**
+   * To be invoked once before each flattened document when streaming JSON into the index
+   */
+  default void startFlattenedDocument() {
+  }
+
+  /**
+   * To be invoked once after each flattened JSON document when streaming JSON into the index
+   */
+  default void endFlattenedDocument() {

Review comment:
       Interface should not have the concept of "flattened" documents as this won't apply to a Tree-based JSON index or maybe even a hash-based JSON index.




-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765947811



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexCreator.java
##########
@@ -29,7 +29,39 @@
   char KEY_VALUE_SEPARATOR = '\0';
 
   /**
-   * Adds the next json value.
+   * To be invoked once before each physical JSON document when streaming JSON into the index
+   * @param numFlattenedDocs the number of flattened documents to be streamed into the index
+   */
+  default void startDocument(int numFlattenedDocs) {
+  }
+
+  /**
+   * To be invoked after each JSON document when streaming JSON into the index.
+   */
+  default void endDocument() {
+  }
+
+  /**
+   * To be invoked once before each flattened document when streaming JSON into the index
+   */
+  default void startFlattenedDocument() {
+  }
+
+  /**
+   * To be invoked once after each flattened JSON document when streaming JSON into the index
+   */
+  default void endFlattenedDocument() {

Review comment:
       Interface should not have the concept of "flattened" documents as this won't apply to a Tree-based JSON index of maybe even a hash-based JSON index.




-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765087431



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexCreator.java
##########
@@ -29,7 +29,39 @@
   char KEY_VALUE_SEPARATOR = '\0';
 
   /**
-   * Adds the next json value.
+   * To be invoked once before each physical JSON document when streaming JSON into the index
+   * @param numFlattenedDocs the number of flattened documents to be streamed into the index
+   */
+  default void startDocument(int numFlattenedDocs) {
+  }
+
+  /**
+   * To be invoked after each physical JSON document when streaming JSON into the index.
+   */
+  default void endDocument() {
+  }
+
+  /**
+   * To be invoked once before each nested (logical) JSON document when streaming JSON into the index

Review comment:
       I don't think this is resolved yet. Please give me some time to look at this further.




-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r764463596



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexCreator.java
##########
@@ -29,7 +29,39 @@
   char KEY_VALUE_SEPARATOR = '\0';
 
   /**
-   * Adds the next json value.
+   * To be invoked once before each physical JSON document when streaming JSON into the index
+   * @param numFlattenedDocs the number of flattened documents to be streamed into the index
+   */
+  default void startDocument(int numFlattenedDocs) {
+  }
+
+  /**
+   * To be invoked after each physical JSON document when streaming JSON into the index.
+   */
+  default void endDocument() {
+  }
+
+  /**
+   * To be invoked once before each nested (logical) JSON document when streaming JSON into the index

Review comment:
       I am a bit confused here. What do you mean by `physical JSON document` and `nested (logical) JSON document`. I thought there was only one kind of JSON document (as defined by JSON spec)?




-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765948713



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexers.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.spi.index.creator;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+
+/**
+ * Registration point to allow overriding of JsonIndexing
+ */
+public final class JsonIndexers {
+
+  private JsonIndexers() {
+  }
+
+  private static final DefaultJsonIndexer DEFAULT_JSON_INDEXER = new DefaultJsonIndexer();
+  private static final Function<JsonIndexer, JsonIndexer> DEFAULT = Function.identity();
+  private static final AtomicReference<Function<JsonIndexer, JsonIndexer>> REGISTERED = new AtomicReference<>(DEFAULT);
+
+  /**
+   * Register a decorator which gets wraps the default JSON indexer so that it can intercept requests to

Review comment:
       I am not seeing the purpose behind `Function<JsonIndexer, JsonIndexer>`, I think that can be avoided. Happy to discuss offline if needed.




-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r764463596



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexCreator.java
##########
@@ -29,7 +29,39 @@
   char KEY_VALUE_SEPARATOR = '\0';
 
   /**
-   * Adds the next json value.
+   * To be invoked once before each physical JSON document when streaming JSON into the index
+   * @param numFlattenedDocs the number of flattened documents to be streamed into the index
+   */
+  default void startDocument(int numFlattenedDocs) {
+  }
+
+  /**
+   * To be invoked after each physical JSON document when streaming JSON into the index.
+   */
+  default void endDocument() {
+  }
+
+  /**
+   * To be invoked once before each nested (logical) JSON document when streaming JSON into the index

Review comment:
       What do you mean by `physical JSON document` and `nested (logical) JSON document`. I thought there was only one kind of JSON document (as defined by JSON spec)?




-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765026000



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexers.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.spi.index.creator;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+
+/**
+ * Registration point to allow overriding of JsonIndexing
+ */
+public final class JsonIndexers {
+
+  private JsonIndexers() {
+  }
+
+  private static final DefaultJsonIndexer DEFAULT_JSON_INDEXER = new DefaultJsonIndexer();
+  private static final Function<JsonIndexer, JsonIndexer> DEFAULT = Function.identity();
+  private static final AtomicReference<Function<JsonIndexer, JsonIndexer>> REGISTERED = new AtomicReference<>(DEFAULT);
+
+  /**
+   * Register a decorator which gets wraps the default JSON indexer so that it can intercept requests to

Review comment:
       We don't need to assume that. Whatever is in the `JSON` datatype column has to be JSON. The reason we want to do this is to make subsequent query processing and indexing simpler (just like only integer values can be in INT columns). If we introduce Protobuf support later on, I really hope that we would do so by introducing a new datatype as well. JSON specification is very specific, so we should not overload the use of term JSON to mean things that JSON specification isn't.
   
   The plugin concept, in my view, would be useful to have different implementations of JSON index. For example a tree based JSON index or a Hashtable based JSON index which could help in optimizing particular types of JSON queries. However, we should keep these a JSON index, separate from ComplexType index, or a Protobuf index.




-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765016800



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexCreator.java
##########
@@ -29,7 +29,39 @@
   char KEY_VALUE_SEPARATOR = '\0';
 
   /**
-   * Adds the next json value.
+   * To be invoked once before each physical JSON document when streaming JSON into the index
+   * @param numFlattenedDocs the number of flattened documents to be streamed into the index
+   */
+  default void startDocument(int numFlattenedDocs) {
+  }
+
+  /**
+   * To be invoked after each physical JSON document when streaming JSON into the index.
+   */
+  default void endDocument() {
+  }
+
+  /**
+   * To be invoked once before each nested (logical) JSON document when streaming JSON into the index

Review comment:
       `flattened` isn't helping here either IMHO. Unless I am missing something there is no such thing as a `physical`, `nested`, or `flattened` JSON and use of such terms and interface functions associated with them will only cause confusion. We just need `start`, `end`, and `add` methods. Anything else is just very specific to your particular implementation of JSON index and should not be put at interface level and may not carry over to other implementations.




-- 
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

[GitHub] [pinot] richardstartin commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765059886



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexCreator.java
##########
@@ -29,7 +29,39 @@
   char KEY_VALUE_SEPARATOR = '\0';
 
   /**
-   * Adds the next json value.
+   * To be invoked once before each physical JSON document when streaming JSON into the index
+   * @param numFlattenedDocs the number of flattened documents to be streamed into the index
+   */
+  default void startDocument(int numFlattenedDocs) {
+  }
+
+  /**
+   * To be invoked after each physical JSON document when streaming JSON into the index.
+   */
+  default void endDocument() {
+  }
+
+  /**
+   * To be invoked once before each nested (logical) JSON document when streaming JSON into the index

Review comment:
       I'm inclined to agree on this point and think I can achieve the same effect with a different interface. I will address this.




-- 
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

[GitHub] [pinot] codecov-commenter commented on pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#issuecomment-988139936


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#7876](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2ada9ae) into [master](https://codecov.io/gh/apache/pinot/commit/c999a23caae3e3769e1729d56da4c5c0de594b31?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c999a23) will **decrease** coverage by `50.67%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7876/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #7876       +/-   ##
   =============================================
   - Coverage     65.07%   14.40%   -50.68%     
   + Complexity     4082       80     -4002     
   =============================================
     Files          1538     1541        +3     
     Lines         79993    80010       +17     
     Branches      12035    12034        -1     
   =============================================
   - Hits          52057    11523    -40534     
   - Misses        24219    67638    +43419     
   + Partials       3717      849     -2868     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests1 | `?` | |
   | unittests2 | `14.40% <0.00%> (-0.01%)` | :arrow_down: |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...nt/creator/impl/inv/json/BaseJsonIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9pbnYvanNvbi9CYXNlSnNvbkluZGV4Q3JlYXRvci5qYXZh) | `0.00% <0.00%> (-93.85%)` | :arrow_down: |
   | [...t/index/loader/invertedindex/JsonIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0pzb25JbmRleEhhbmRsZXIuamF2YQ==) | `0.00% <0.00%> (-72.89%)` | :arrow_down: |
   | [.../segment/spi/index/creator/DefaultJsonIndexer.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvRGVmYXVsdEpzb25JbmRleGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...ot/segment/spi/index/creator/JsonIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvSnNvbkluZGV4Q3JlYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [.../pinot/segment/spi/index/creator/JsonIndexers.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvSnNvbkluZGV4ZXJzLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [1053 more](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c999a23...2ada9ae](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765026000



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexers.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.spi.index.creator;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+
+/**
+ * Registration point to allow overriding of JsonIndexing
+ */
+public final class JsonIndexers {
+
+  private JsonIndexers() {
+  }
+
+  private static final DefaultJsonIndexer DEFAULT_JSON_INDEXER = new DefaultJsonIndexer();
+  private static final Function<JsonIndexer, JsonIndexer> DEFAULT = Function.identity();
+  private static final AtomicReference<Function<JsonIndexer, JsonIndexer>> REGISTERED = new AtomicReference<>(DEFAULT);
+
+  /**
+   * Register a decorator which gets wraps the default JSON indexer so that it can intercept requests to

Review comment:
       We don't need to assume that. Whatever is in the `JSON` datatype column has to be JSON. The reason we want to do this is to make subsequent query processing and indexing simpler. If we introduce Protobuf support later on, I really hope that we would do so by introducing a new datatype as well. JSON specification is very specific, so we should not overload the use of term JSON to mean things that JSON specification isn't.
   
   The plugin concept, in my view, would be useful to have different implementations of JSON index. For example a tree based JSON index or a Hashtable based JSON index which could help in optimizing particular types of JSON queries. However, we should keep these a JSON index, separate from ComplexType index, or a Protobuf index.




-- 
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

[GitHub] [pinot] richardstartin commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765043934



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexers.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.spi.index.creator;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+
+/**
+ * Registration point to allow overriding of JsonIndexing
+ */
+public final class JsonIndexers {
+
+  private JsonIndexers() {
+  }
+
+  private static final DefaultJsonIndexer DEFAULT_JSON_INDEXER = new DefaultJsonIndexer();
+  private static final Function<JsonIndexer, JsonIndexer> DEFAULT = Function.identity();
+  private static final AtomicReference<Function<JsonIndexer, JsonIndexer>> REGISTERED = new AtomicReference<>(DEFAULT);
+
+  /**
+   * Register a decorator which gets wraps the default JSON indexer so that it can intercept requests to

Review comment:
       I don't think that the JSON data type constrains the physical representation of the JSON. With the right plugins, I should be able to store destructured JSON in a JSON column, but if I have destructured JSON, I might not want to rematerialise it as JSON just so it can be indexed.




-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765080186



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexers.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.spi.index.creator;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+
+/**
+ * Registration point to allow overriding of JsonIndexing
+ */
+public final class JsonIndexers {
+
+  private JsonIndexers() {
+  }
+
+  private static final DefaultJsonIndexer DEFAULT_JSON_INDEXER = new DefaultJsonIndexer();
+  private static final Function<JsonIndexer, JsonIndexer> DEFAULT = Function.identity();
+  private static final AtomicReference<Function<JsonIndexer, JsonIndexer>> REGISTERED = new AtomicReference<>(DEFAULT);
+
+  /**
+   * Register a decorator which gets wraps the default JSON indexer so that it can intercept requests to

Review comment:
       It seems like you have more things in mind than what is coming through across this PR and your last PR on JSON. It almost seems like we should have a design document here so that the path you are perusing is clear.
   
   My original question still remains unanswered: Why do we need to intercept requests to index JSON column? Just registering the indexer should be sufficient, right?
   
   > With the right plugins, I should be able to store destructured JSON in a JSON column
   
   With the right "storage" plugin, you could store destructured JSON into a JSON column or store destructed JSON into multiple columns, etc. An "indexing" plugin may then be associated with a corresponding storage plugin if needed. (and still not sure where protobuf comes into this).
   
   I really think we need to take the time to create a design document here, as to what you are trying to do and what the end goal is, before we move forward with further changes.




-- 
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

[GitHub] [pinot] richardstartin closed pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
richardstartin closed pull request #7876:
URL: https://github.com/apache/pinot/pull/7876


   


-- 
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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#issuecomment-988139936


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#7876](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2ada9ae) into [master](https://codecov.io/gh/apache/pinot/commit/c999a23caae3e3769e1729d56da4c5c0de594b31?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c999a23) will **increase** coverage by `6.35%`.
   > The diff coverage is `68.29%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7876/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #7876      +/-   ##
   ============================================
   + Coverage     65.07%   71.42%   +6.35%     
   + Complexity     4082     4081       -1     
   ============================================
     Files          1538     1586      +48     
     Lines         79993    81880    +1887     
     Branches      12035    12237     +202     
   ============================================
   + Hits          52057    58486    +6429     
   + Misses        24219    19437    -4782     
   - Partials       3717     3957     +240     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `29.24% <0.00%> (?)` | |
   | integration2 | `27.69% <0.00%> (?)` | |
   | unittests1 | `68.46% <68.29%> (-0.03%)` | :arrow_down: |
   | unittests2 | `14.40% <0.00%> (-0.01%)` | :arrow_down: |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...t/index/loader/invertedindex/JsonIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0pzb25JbmRleEhhbmRsZXIuamF2YQ==) | `75.00% <40.00%> (+2.11%)` | :arrow_up: |
   | [...ot/segment/spi/index/creator/JsonIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvSnNvbkluZGV4Q3JlYXRvci5qYXZh) | `50.00% <50.00%> (ø)` | |
   | [.../segment/spi/index/creator/DefaultJsonIndexer.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvRGVmYXVsdEpzb25JbmRleGVyLmphdmE=) | `54.54% <54.54%> (ø)` | |
   | [.../pinot/segment/spi/index/creator/JsonIndexers.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvSnNvbkluZGV4ZXJzLmphdmE=) | `71.42% <71.42%> (ø)` | |
   | [...nt/creator/impl/inv/json/BaseJsonIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9pbnYvanNvbi9CYXNlSnNvbkluZGV4Q3JlYXRvci5qYXZh) | `94.02% <92.85%> (+0.18%)` | :arrow_up: |
   | [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
   | [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
   | [...cal/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9idWlsZGVyL0Jhc2VTaW5nbGVUcmVlQnVpbGRlci5qYXZh) | `88.28% <0.00%> (-0.91%)` | :arrow_down: |
   | [...g/apache/pinot/server/api/resources/ErrorInfo.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9FcnJvckluZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [.../plugin/inputformat/csv/CSVRecordReaderConfig.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1pbnB1dC1mb3JtYXQvcGlub3QtY3N2L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wbHVnaW4vaW5wdXRmb3JtYXQvY3N2L0NTVlJlY29yZFJlYWRlckNvbmZpZy5qYXZh) | `0.00% <0.00%> (ø)` | |
   | ... and [363 more](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c999a23...2ada9ae](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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

[GitHub] [pinot] richardstartin commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r764753118



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexers.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.spi.index.creator;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+
+/**
+ * Registration point to allow overriding of JsonIndexing
+ */
+public final class JsonIndexers {
+
+  private JsonIndexers() {
+  }
+
+  private static final DefaultJsonIndexer DEFAULT_JSON_INDEXER = new DefaultJsonIndexer();
+  private static final Function<JsonIndexer, JsonIndexer> DEFAULT = Function.identity();
+  private static final AtomicReference<Function<JsonIndexer, JsonIndexer>> REGISTERED = new AtomicReference<>(DEFAULT);
+
+  /**
+   * Register a decorator which gets wraps the default JSON indexer so that it can intercept requests to

Review comment:
       Currently its assumed that whatever is in the JSON column is actually JSON, but the indexing technique can be made generic (so that you could have a forward index containing, say, Protobuf records stored in bytes, and then a JSON index so long as you have a plugin which can construct the appropriate paths) with the layer of indirection provided in 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

[GitHub] [pinot] richardstartin commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765095272



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexers.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.spi.index.creator;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+
+/**
+ * Registration point to allow overriding of JsonIndexing
+ */
+public final class JsonIndexers {
+
+  private JsonIndexers() {
+  }
+
+  private static final DefaultJsonIndexer DEFAULT_JSON_INDEXER = new DefaultJsonIndexer();
+  private static final Function<JsonIndexer, JsonIndexer> DEFAULT = Function.identity();
+  private static final AtomicReference<Function<JsonIndexer, JsonIndexer>> REGISTERED = new AtomicReference<>(DEFAULT);
+
+  /**
+   * Register a decorator which gets wraps the default JSON indexer so that it can intercept requests to

Review comment:
       > My original question still remains unanswered: Why do we need to intercept requests to index JSON column? Just registering the indexer should be sufficient, right?
   
   I tried to answer this by way of example. Currently it is assumed that `ForwardIndexReader#getString` returns a JSON document for a JSON column, which constrains the physical representation. This PR provides a pluggable mediation mechanism between the `ForwardIndexReader` and the `JsonIndexCreator`.




-- 
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

[GitHub] [pinot] richardstartin commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r765045233



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexCreator.java
##########
@@ -29,7 +29,39 @@
   char KEY_VALUE_SEPARATOR = '\0';
 
   /**
-   * Adds the next json value.
+   * To be invoked once before each physical JSON document when streaming JSON into the index
+   * @param numFlattenedDocs the number of flattened documents to be streamed into the index
+   */
+  default void startDocument(int numFlattenedDocs) {
+  }
+
+  /**
+   * To be invoked after each physical JSON document when streaming JSON into the index.
+   */
+  default void endDocument() {
+  }
+
+  /**
+   * To be invoked once before each nested (logical) JSON document when streaming JSON into the index

Review comment:
       Without understanding the concept of flattening it would be impossible to implement the existing JSON index, which is why it is modelled as a first class concept. 




-- 
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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#issuecomment-988139936


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#7876](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2ada9ae) into [master](https://codecov.io/gh/apache/pinot/commit/c999a23caae3e3769e1729d56da4c5c0de594b31?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c999a23) will **increase** coverage by `5.11%`.
   > The diff coverage is `68.29%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7876/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #7876      +/-   ##
   ============================================
   + Coverage     65.07%   70.18%   +5.11%     
   + Complexity     4082     4081       -1     
   ============================================
     Files          1538     1586      +48     
     Lines         79993    81880    +1887     
     Branches      12035    12237     +202     
   ============================================
   + Hits          52057    57471    +5414     
   + Misses        24219    20467    -3752     
   - Partials       3717     3942     +225     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration2 | `27.69% <0.00%> (?)` | |
   | unittests1 | `68.46% <68.29%> (-0.03%)` | :arrow_down: |
   | unittests2 | `14.40% <0.00%> (-0.01%)` | :arrow_down: |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...t/index/loader/invertedindex/JsonIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0pzb25JbmRleEhhbmRsZXIuamF2YQ==) | `75.00% <40.00%> (+2.11%)` | :arrow_up: |
   | [...ot/segment/spi/index/creator/JsonIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvSnNvbkluZGV4Q3JlYXRvci5qYXZh) | `50.00% <50.00%> (ø)` | |
   | [.../segment/spi/index/creator/DefaultJsonIndexer.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvRGVmYXVsdEpzb25JbmRleGVyLmphdmE=) | `54.54% <54.54%> (ø)` | |
   | [.../pinot/segment/spi/index/creator/JsonIndexers.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvSnNvbkluZGV4ZXJzLmphdmE=) | `71.42% <71.42%> (ø)` | |
   | [...nt/creator/impl/inv/json/BaseJsonIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9pbnYvanNvbi9CYXNlSnNvbkluZGV4Q3JlYXRvci5qYXZh) | `94.02% <92.85%> (+0.18%)` | :arrow_up: |
   | [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `67.83% <0.00%> (-14.69%)` | :arrow_down: |
   | [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
   | [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
   | [...cal/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9idWlsZGVyL0Jhc2VTaW5nbGVUcmVlQnVpbGRlci5qYXZh) | `88.28% <0.00%> (-0.91%)` | :arrow_down: |
   | [...va/org/apache/pinot/controller/ControllerConf.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyQ29uZi5qYXZh) | `56.50% <0.00%> (ø)` | |
   | ... and [328 more](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c999a23...2ada9ae](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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

[GitHub] [pinot] amrishlal commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r764464702



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexers.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.spi.index.creator;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+
+/**
+ * Registration point to allow overriding of JsonIndexing
+ */
+public final class JsonIndexers {
+
+  private JsonIndexers() {
+  }
+
+  private static final DefaultJsonIndexer DEFAULT_JSON_INDEXER = new DefaultJsonIndexer();
+  private static final Function<JsonIndexer, JsonIndexer> DEFAULT = Function.identity();
+  private static final AtomicReference<Function<JsonIndexer, JsonIndexer>> REGISTERED = new AtomicReference<>(DEFAULT);
+
+  /**
+   * Register a decorator which gets wraps the default JSON indexer so that it can intercept requests to

Review comment:
       Why do we need to intercept requests to index JSON column? Just registering the indexer should be sufficient, right?




-- 
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

[GitHub] [pinot] richardstartin commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r764754633



##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/JsonIndexCreator.java
##########
@@ -29,7 +29,39 @@
   char KEY_VALUE_SEPARATOR = '\0';
 
   /**
-   * Adds the next json value.
+   * To be invoked once before each physical JSON document when streaming JSON into the index
+   * @param numFlattenedDocs the number of flattened documents to be streamed into the index
+   */
+  default void startDocument(int numFlattenedDocs) {
+  }
+
+  /**
+   * To be invoked after each physical JSON document when streaming JSON into the index.
+   */
+  default void endDocument() {
+  }
+
+  /**
+   * To be invoked once before each nested (logical) JSON document when streaming JSON into the index

Review comment:
       By physical document I mean the actual JSON document being indexed. By nested (logical) JSON document I mean one of the flattened documents which actually gets indexed by the JSON index. Perhaps physical and logical aren't helping 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

[GitHub] [pinot] richardstartin commented on a change in pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#discussion_r764202584



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/json/BaseJsonIndexCreator.java
##########
@@ -67,8 +66,6 @@
   final File _invertedIndexFile;
   final IntList _numFlattenedRecordsList = new IntArrayList();
   final Map<String, RoaringBitmapWriter<RoaringBitmap>> _postingListMap = new TreeMap<>();
-  final RoaringBitmapWriter.Wizard<Container, RoaringBitmap> _bitmapWriterWizard =
-      RoaringBitmapWriter.writer();

Review comment:
       I noticed this usage of non public API (the inner class itself is only visible because the project isn't modular yet)




-- 
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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #7876: JSON indexing adapter

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7876:
URL: https://github.com/apache/pinot/pull/7876#issuecomment-988139936


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#7876](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2ada9ae) into [master](https://codecov.io/gh/apache/pinot/commit/c999a23caae3e3769e1729d56da4c5c0de594b31?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c999a23) will **decrease** coverage by `0.02%`.
   > The diff coverage is `68.29%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7876/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #7876      +/-   ##
   ============================================
   - Coverage     65.07%   65.05%   -0.03%     
   + Complexity     4082     4081       -1     
   ============================================
     Files          1538     1541       +3     
     Lines         79993    80010      +17     
     Branches      12035    12034       -1     
   ============================================
   - Hits          52057    52049       -8     
   - Misses        24219    24248      +29     
   + Partials       3717     3713       -4     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests1 | `68.46% <68.29%> (-0.03%)` | :arrow_down: |
   | unittests2 | `14.40% <0.00%> (-0.01%)` | :arrow_down: |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...t/index/loader/invertedindex/JsonIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0pzb25JbmRleEhhbmRsZXIuamF2YQ==) | `75.00% <40.00%> (+2.11%)` | :arrow_up: |
   | [...ot/segment/spi/index/creator/JsonIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvSnNvbkluZGV4Q3JlYXRvci5qYXZh) | `50.00% <50.00%> (ø)` | |
   | [.../segment/spi/index/creator/DefaultJsonIndexer.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvRGVmYXVsdEpzb25JbmRleGVyLmphdmE=) | `54.54% <54.54%> (ø)` | |
   | [.../pinot/segment/spi/index/creator/JsonIndexers.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvSnNvbkluZGV4ZXJzLmphdmE=) | `71.42% <71.42%> (ø)` | |
   | [...nt/creator/impl/inv/json/BaseJsonIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9pbnYvanNvbi9CYXNlSnNvbkluZGV4Q3JlYXRvci5qYXZh) | `94.02% <92.85%> (+0.18%)` | :arrow_up: |
   | [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `67.13% <0.00%> (-15.39%)` | :arrow_down: |
   | [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
   | [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
   | [...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `79.05% <0.00%> (-1.05%)` | :arrow_down: |
   | [...cal/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9idWlsZGVyL0Jhc2VTaW5nbGVUcmVlQnVpbGRlci5qYXZh) | `88.28% <0.00%> (-0.91%)` | :arrow_down: |
   | ... and [4 more](https://codecov.io/gh/apache/pinot/pull/7876/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c999a23...2ada9ae](https://codecov.io/gh/apache/pinot/pull/7876?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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