Jackie-Jiang commented on code in PR #12683:
URL: https://github.com/apache/pinot/pull/12683#discussion_r1534379709


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/json/MutableJsonIndexImpl.java:
##########
@@ -367,10 +379,18 @@ public void convertFlattenedDocIdsToDocIds(Map<String, 
RoaringBitmap> valueToFla
   }
 
   @Override
-  public Map<String, RoaringBitmap> getMatchingFlattenedDocsMap(String 
jsonPathKey) {
+  public Map<String, RoaringBitmap> getMatchingFlattenedDocsMap(String 
jsonPathKey, @Nullable String filterString) {

Review Comment:
   Not introduced in this PR, but I think we should support json path key with 
and without `$` prefix. See `getMatchingFlattenedDocIds()` for reference



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/json/MutableJsonIndexImpl.java:
##########
@@ -167,30 +168,41 @@ private boolean isExclusive(Predicate.Type predicateType) 
{
    * Returns the matching flattened doc ids for the given filter.
    */
   private RoaringBitmap getMatchingFlattenedDocIds(FilterContext filter) {
+    return getMatchingFlattenedDocIds(filter, false);
+  }
+
+  private RoaringBitmap getMatchingFlattenedDocIds(FilterContext filter, 
boolean allowNestedExclusivePredicate) {

Review Comment:
   Trying to understand why we cannot always allow nested exclusive predicate?



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/json/MutableJsonIndexImpl.java:
##########
@@ -167,30 +168,41 @@ private boolean isExclusive(Predicate.Type predicateType) 
{
    * Returns the matching flattened doc ids for the given filter.
    */
   private RoaringBitmap getMatchingFlattenedDocIds(FilterContext filter) {
+    return getMatchingFlattenedDocIds(filter, false);
+  }
+
+  private RoaringBitmap getMatchingFlattenedDocIds(FilterContext filter, 
boolean allowNestedExclusivePredicate) {
     switch (filter.getType()) {
       case AND: {
         List<FilterContext> children = filter.getChildren();
         int numChildren = children.size();
-        RoaringBitmap matchingDocIds = 
getMatchingFlattenedDocIds(children.get(0));
+        RoaringBitmap matchingDocIds = 
getMatchingFlattenedDocIds(children.get(0), allowNestedExclusivePredicate);
         for (int i = 1; i < numChildren; i++) {
-          matchingDocIds.and(getMatchingFlattenedDocIds(children.get(i)));
+          matchingDocIds.and(getMatchingFlattenedDocIds(children.get(i), 
allowNestedExclusivePredicate));
         }
         return matchingDocIds;
       }
       case OR: {
         List<FilterContext> children = filter.getChildren();
         int numChildren = children.size();
-        RoaringBitmap matchingDocIds = 
getMatchingFlattenedDocIds(children.get(0));
+        RoaringBitmap matchingDocIds = 
getMatchingFlattenedDocIds(children.get(0), allowNestedExclusivePredicate);
         for (int i = 1; i < numChildren; i++) {
-          matchingDocIds.or(getMatchingFlattenedDocIds(children.get(i)));
+          matchingDocIds.or(getMatchingFlattenedDocIds(children.get(i), 
allowNestedExclusivePredicate));
         }
         return matchingDocIds;
       }
       case PREDICATE: {

Review Comment:
   Should we add `NOT` since we allow exclusive predicate?



-- 
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-unsubscr...@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to