You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/01/14 03:50:56 UTC

[GitHub] [druid] gianm opened a new pull request #9181: Speed up String first/last aggregators when folding isn't needed.

gianm opened a new pull request #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181
 
 
   Examines the value column, and disables fold checking via a needsFoldCheck
   flag if that column can't possibly contain SerializableLongStringPairs. This
   is helpful because it avoids calling getObject on the value selector when
   unnecessary; say, because the time selector didn't yield an earlier or later
   value.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] lgtm-com[bot] commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#issuecomment-575439630
 
 
   This pull request **fixes 1 alert** when merging de0697cb1834f77a2fafc57e5d56673a558c5e83 into 486c0fd149d9837a64550ecb9e85d9b6cd4beb24 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-cbc76f7454ddad92381f9db32c521dcbd504afb8)
   
   **fixed alerts:**
   
   * 1 for Useless null check

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] gianm commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#discussion_r367656434
 
 

 ##########
 File path: processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstLastUtils.java
 ##########
 @@ -33,23 +36,63 @@
 {
   private static final int NULL_VALUE = -1;
 
+  /**
+   * Shorten "s" to "maxBytes" chars. Fast and loose because these are *chars* not *bytes*. Use
+   * {@link #chop(String, int)} for slower, but accurate chopping.
+   */
+  @Nullable
+  public static String fastLooseChop(@Nullable final String s, final int maxBytes)
+  {
+    if (s == null || s.length() <= maxBytes) {
+      return s;
+    } else {
+      return s.substring(0, maxBytes);
+    }
+  }
+
+  /**
+   * Shorten "s" to what could fit in "maxBytes" bytes as UTF-8.
+   */
   @Nullable
   public static String chop(@Nullable final String s, final int maxBytes)
   {
     if (s == null) {
       return null;
     } else {
-      // Shorten firstValue to what could fit in maxBytes as UTF-8.
       final byte[] bytes = new byte[maxBytes];
       final int len = StringUtils.toUtf8WithLimit(s, ByteBuffer.wrap(bytes));
       return new String(bytes, 0, len, StandardCharsets.UTF_8);
     }
   }
 
+  /**
+   * Returns whether a given value selector *might* contain SerializablePairLongString objects.
+   */
+  public static boolean selectorNeedsFoldCheck(
+      final BaseObjectColumnValueSelector<?> valueSelector,
+      @Nullable final ColumnCapabilities valueSelectorCapabilities
+  )
+  {
+    if (valueSelectorCapabilities != null && valueSelectorCapabilities.getType() != ValueType.COMPLEX) {
+      // Known, non-complex type.
+      return false;
+    }
+
+    if (valueSelector instanceof NilColumnValueSelector) {
+      // Nil column, definitely no SerializablePairLongStrings.
+      return false;
+    }
+
+    // Check if the reported class could possibly be SerializablePairLongString.
+    final Class<?> clazz = valueSelector.classOfObject();
+    return clazz.isAssignableFrom(SerializablePairLongString.class)
 
 Review comment:
   I changed it to:
   
   ```java
       // Check if the selector class could possibly be a SerializablePairLongString (either a superclass or subclass).
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#discussion_r367736165
 
 

 ##########
 File path: core/src/test/java/org/apache/druid/java/util/common/StringUtilsTest.java
 ##########
 @@ -246,4 +246,32 @@ public void testRpad()
     Assert.assertEquals(s5, null);
   }
 
+  @Test
+  public void testChop()
+  {
+    Assert.assertEquals("foo", StringUtils.chop("foo", 5));
+    Assert.assertEquals("fo", StringUtils.chop("foo", 2));
+    Assert.assertEquals("", StringUtils.chop("foo", 0));
+    Assert.assertEquals("smile 🙂 for", StringUtils.chop("smile 🙂 for the camera", 14));
 
 Review comment:
   🙂

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] gianm commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#discussion_r367656532
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/query/aggregation/last/StringLastBufferAggregatorTest.java
 ##########
 @@ -81,6 +82,43 @@ public void testBufferAggregate()
 
   }
 
+  @Test
+  public void testBufferAggregateWithFoldCheck()
+  {
+    final long[] timestamps = {1526724600L, 1526724700L, 1526724800L, 1526725900L, 1526725000L};
+    final String[] strings = {"AAAA", "BBBB", "CCCC", "DDDD", "EEEE"};
+    Integer maxStringBytes = 1024;
+
+    TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps);
+    TestObjectColumnSelector<String> objectColumnSelector = new TestObjectColumnSelector<>(strings);
+
+    StringLastAggregatorFactory factory = new StringLastAggregatorFactory(
+        "billy", "billy", maxStringBytes
+    );
+
+    StringLastBufferAggregator agg = new StringLastBufferAggregator(
+        longColumnSelector,
+        objectColumnSelector,
+        maxStringBytes,
+        true
+    );
+
+    ByteBuffer buf = ByteBuffer.allocate(factory.getMaxIntermediateSize());
+    int position = 0;
+
+    agg.init(buf, position);
+    //noinspection ForLoopReplaceableByForEach
+    for (int i = 0; i < timestamps.length; i++) {
+      aggregateBuffer(longColumnSelector, objectColumnSelector, agg, buf, position);
+    }
+
+    SerializablePairLongString sp = ((SerializablePairLongString) agg.get(buf, position));
+
+
+    Assert.assertEquals("expectec last string value", "DDDD", sp.rhs);
 
 Review comment:
   Updated.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#discussion_r366825374
 
 

 ##########
 File path: processing/src/test/java/org/apache/druid/query/aggregation/last/StringLastBufferAggregatorTest.java
 ##########
 @@ -81,6 +82,43 @@ public void testBufferAggregate()
 
   }
 
+  @Test
+  public void testBufferAggregateWithFoldCheck()
+  {
+    final long[] timestamps = {1526724600L, 1526724700L, 1526724800L, 1526725900L, 1526725000L};
+    final String[] strings = {"AAAA", "BBBB", "CCCC", "DDDD", "EEEE"};
+    Integer maxStringBytes = 1024;
+
+    TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps);
+    TestObjectColumnSelector<String> objectColumnSelector = new TestObjectColumnSelector<>(strings);
+
+    StringLastAggregatorFactory factory = new StringLastAggregatorFactory(
+        "billy", "billy", maxStringBytes
+    );
+
+    StringLastBufferAggregator agg = new StringLastBufferAggregator(
+        longColumnSelector,
+        objectColumnSelector,
+        maxStringBytes,
+        true
+    );
+
+    ByteBuffer buf = ByteBuffer.allocate(factory.getMaxIntermediateSize());
+    int position = 0;
+
+    agg.init(buf, position);
+    //noinspection ForLoopReplaceableByForEach
+    for (int i = 0; i < timestamps.length; i++) {
+      aggregateBuffer(longColumnSelector, objectColumnSelector, agg, buf, position);
+    }
+
+    SerializablePairLongString sp = ((SerializablePairLongString) agg.get(buf, position));
+
+
+    Assert.assertEquals("expectec last string value", "DDDD", sp.rhs);
 
 Review comment:
   nit: typo 'expectec'

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis merged pull request #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
clintropolis merged pull request #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] lgtm-com[bot] commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#issuecomment-573999395
 
 
   This pull request **fixes 1 alert** when merging ecc43107320489ea1267aed7b2dbd30f440104f9 into ea51bc45bff8b0b5c373d1121fa88b6d507fcabe - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-ac29c6587c91acbc15305f2cecfde8dc594497ae)
   
   **fixed alerts:**
   
   * 1 for Useless null check

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jon-wei commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
jon-wei commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#issuecomment-575418815
 
 
   There's a TC error about an resolved reference to the chop method

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] gianm commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
gianm commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#issuecomment-575353132
 
 
   @clintropolis Thanks for reviewing. I updated the patch to reflect your comments.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#discussion_r366814361
 
 

 ##########
 File path: processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstLastUtils.java
 ##########
 @@ -33,23 +36,63 @@
 {
   private static final int NULL_VALUE = -1;
 
+  /**
+   * Shorten "s" to "maxBytes" chars. Fast and loose because these are *chars* not *bytes*. Use
+   * {@link #chop(String, int)} for slower, but accurate chopping.
+   */
+  @Nullable
+  public static String fastLooseChop(@Nullable final String s, final int maxBytes)
+  {
+    if (s == null || s.length() <= maxBytes) {
+      return s;
+    } else {
+      return s.substring(0, maxBytes);
+    }
+  }
+
+  /**
+   * Shorten "s" to what could fit in "maxBytes" bytes as UTF-8.
+   */
   @Nullable
   public static String chop(@Nullable final String s, final int maxBytes)
   {
     if (s == null) {
       return null;
     } else {
-      // Shorten firstValue to what could fit in maxBytes as UTF-8.
       final byte[] bytes = new byte[maxBytes];
       final int len = StringUtils.toUtf8WithLimit(s, ByteBuffer.wrap(bytes));
       return new String(bytes, 0, len, StandardCharsets.UTF_8);
     }
   }
 
+  /**
+   * Returns whether a given value selector *might* contain SerializablePairLongString objects.
+   */
+  public static boolean selectorNeedsFoldCheck(
+      final BaseObjectColumnValueSelector<?> valueSelector,
+      @Nullable final ColumnCapabilities valueSelectorCapabilities
+  )
+  {
+    if (valueSelectorCapabilities != null && valueSelectorCapabilities.getType() != ValueType.COMPLEX) {
+      // Known, non-complex type.
+      return false;
+    }
+
+    if (valueSelector instanceof NilColumnValueSelector) {
+      // Nil column, definitely no SerializablePairLongStrings.
+      return false;
+    }
+
+    // Check if the reported class could possibly be SerializablePairLongString.
+    final Class<?> clazz = valueSelector.classOfObject();
+    return clazz.isAssignableFrom(SerializablePairLongString.class)
 
 Review comment:
   Hmm, it looks to me like this method only returns true if a selector definitely is a `SerializablePairLongString `. Should this just be returning true if we made it here?
   
   Alternatively, should `needsFoldCheck` be rebranded as `isFold` and the `aggregate` methods of the first last heap and buffer aggs just be reading a `SerializablePairLongString` from the value selector directly instead of calling `readPairFromSelectors`?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] gianm commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
gianm commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#issuecomment-575427555
 
 
   > There's a TC error about an unresolved reference to the chop method
   
   That was from a javadoc for `fastLooseChop`. It looks like `chop` was moved to StringUtils, so I moved `fastLooseChop` to the same place. And added unit tests for good measure.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] lgtm-com[bot] commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#issuecomment-575400116
 
 
   This pull request **fixes 1 alert** when merging c56d895caf30f0b3171ea5cc09615e551adeeae4 into 42359c93dd53f16e52ed79dcd8b63829f4bf2f7b - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-60ba177285eee825f576c2665f6a7661b4aff17a)
   
   **fixed alerts:**
   
   * 1 for Useless null check

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] lgtm-com[bot] commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on issue #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#issuecomment-575378497
 
 
   This pull request **fixes 1 alert** when merging 92f2218cf771c70b1173264e96621850bead8ea8 into a87db7f353cdee4dfa9b541063f59d67706d1b07 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-cd9a4712d6ee17be67a5574f81c981254ad0052b)
   
   **fixed alerts:**
   
   * 1 for Useless null check

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jon-wei edited a comment on issue #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
jon-wei edited a comment on issue #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#issuecomment-575418815
 
 
   There's a TC error about an unresolved reference to the chop method

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.

Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9181: Speed up String first/last aggregators when folding isn't needed.
URL: https://github.com/apache/druid/pull/9181#discussion_r367234568
 
 

 ##########
 File path: processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstLastUtils.java
 ##########
 @@ -33,23 +36,63 @@
 {
   private static final int NULL_VALUE = -1;
 
+  /**
+   * Shorten "s" to "maxBytes" chars. Fast and loose because these are *chars* not *bytes*. Use
+   * {@link #chop(String, int)} for slower, but accurate chopping.
+   */
+  @Nullable
+  public static String fastLooseChop(@Nullable final String s, final int maxBytes)
+  {
+    if (s == null || s.length() <= maxBytes) {
+      return s;
+    } else {
+      return s.substring(0, maxBytes);
+    }
+  }
+
+  /**
+   * Shorten "s" to what could fit in "maxBytes" bytes as UTF-8.
+   */
   @Nullable
   public static String chop(@Nullable final String s, final int maxBytes)
   {
     if (s == null) {
       return null;
     } else {
-      // Shorten firstValue to what could fit in maxBytes as UTF-8.
       final byte[] bytes = new byte[maxBytes];
       final int len = StringUtils.toUtf8WithLimit(s, ByteBuffer.wrap(bytes));
       return new String(bytes, 0, len, StandardCharsets.UTF_8);
     }
   }
 
+  /**
+   * Returns whether a given value selector *might* contain SerializablePairLongString objects.
+   */
+  public static boolean selectorNeedsFoldCheck(
+      final BaseObjectColumnValueSelector<?> valueSelector,
+      @Nullable final ColumnCapabilities valueSelectorCapabilities
+  )
+  {
+    if (valueSelectorCapabilities != null && valueSelectorCapabilities.getType() != ValueType.COMPLEX) {
+      // Known, non-complex type.
+      return false;
+    }
+
+    if (valueSelector instanceof NilColumnValueSelector) {
+      // Nil column, definitely no SerializablePairLongStrings.
+      return false;
+    }
+
+    // Check if the reported class could possibly be SerializablePairLongString.
+    final Class<?> clazz = valueSelector.classOfObject();
+    return clazz.isAssignableFrom(SerializablePairLongString.class)
 
 Review comment:
   Apologies, I was wrong, I think this line + comment just isn't super intuitive on first glance that it is a catch-all that any ‘unknown’ selector types will also be true (since clazz will be Object.class in that case). Maybe expanding on the comment would make it require less thought?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org