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