gianm commented on code in PR #19262:
URL: https://github.com/apache/druid/pull/19262#discussion_r3042967398


##########
processing/src/main/java/org/apache/druid/segment/VirtualColumn.java:
##########
@@ -472,6 +473,24 @@ default ColumnCapabilities capabilities(ColumnInspector 
inspector, String column
    */
   List<String> requiredColumns();
 
+
+  default boolean supportsRequiredRewrite()
+  {
+    return false;
+  }
+
+  /**
+   * Return a copy of this virtual column that is identical to this virtual 
column except that it operates on different
+   * columns, based on a renaming map where the key is the column to be 
renamed and the value is the new column.

Review Comment:
   Useful to say that not all virtual columns implement this, and callers must 
check `supportsRequiredRewrite`.



##########
processing/src/main/java/org/apache/druid/segment/VirtualColumn.java:
##########
@@ -472,6 +473,24 @@ default ColumnCapabilities capabilities(ColumnInspector 
inspector, String column
    */
   List<String> requiredColumns();
 
+
+  default boolean supportsRequiredRewrite()

Review Comment:
   Somewhat obvious what this means, but it would still be good to have a 
javadoc.



##########
processing/src/main/java/org/apache/druid/query/filter/FilterSegmentPruner.java:
##########
@@ -161,4 +166,14 @@ public String toString()
            ", virtualColumns=" + virtualColumns +
            '}';
   }
+
+  @Nullable
+  private VirtualColumn getQueryEquivalent(VirtualColumns shardVirtualColumns, 
VirtualColumn shardVirtualColumn)
+  {
+    final Optional<VirtualColumn> cached = 
shardEquivalenceCache.computeIfAbsent(
+        shardVirtualColumn,

Review Comment:
   Is it ok that the key is just `shardVirtualColumn` even though the `compute` 
function uses `shardVirtualColumns` as well? Can this be called with two 
different `shardVirtualColumns` for the same `FilterSegmentPruner` object and 
get some incorrect cache hits?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to