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