yashmayya commented on code in PR #13835:
URL: https://github.com/apache/pinot/pull/13835#discussion_r1720966644


##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/startree/AggregationFunctionColumn.java:
##########
@@ -127,9 +149,14 @@ public boolean equals(Object obj) {
     if (this == obj) {
       return true;
     }
-    if (obj instanceof AggregationFunctionColumnPair) {
-      AggregationFunctionColumnPair anotherPair = 
(AggregationFunctionColumnPair) obj;
-      return _functionType == anotherPair._functionType && 
_column.equals(anotherPair._column);
+    if (obj instanceof AggregationFunctionColumn) {
+      AggregationFunctionColumn other = (AggregationFunctionColumn) obj;
+      // TODO: Revisit this since it means that for aggregation functions 
where a certain config parameter need not be
+      // checked to determine whether a query can be served by a star-tree 
index, we won't rebuild a star-tree index
+      // if the parameter value is changed in the index configuration.
+      return _functionType == other._functionType && 
_column.equals(other._column)
+          && 
AggregationFunctionType.compareFunctionParametersForStarTree(_functionType, 
_functionParameters,
+          other._functionParameters) == 0;

Review Comment:
   This might not be too big of an issue because it can be easily worked around 
by deleting the index and re-creating with different parameters. While this 
also means that multiple star-tree indexes on the same function / column pair 
but with different parameter values won't be supported for functions outside of 
`DISTINCTCOUNTHLL` / `DISTINCTCOUNTHLLPLUS` etc. I'm not sure there are 
legitimate use cases for that anyway.



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