kgyrtkirk commented on code in PR #17565:
URL: https://github.com/apache/druid/pull/17565#discussion_r1883491417
##########
processing/src/main/java/org/apache/druid/query/topn/TopNQueryEngine.java:
##########
@@ -275,7 +275,7 @@ private static boolean canUsePooledAlgorithm(
final int numBytesToWorkWith = resultsBuf.capacity();
final int numValuesPerPass = numBytesPerRecord > 0 ? numBytesToWorkWith
/ numBytesPerRecord : cardinality;
- return numValuesPerPass <= cardinality;
+ return numValuesPerPass >= cardinality;
Review Comment:
this comparision change caught my eye...the new cmp seem to be more in line
with the intention - but it seems to me that
depending on some conditions
[here](https://github.com/apache/druid/blob/24e5d8a9e81fffd656e9544ae16bea53d7e60808/processing/src/main/java/org/apache/druid/query/topn/TopNQueryEngine.java#L111-L115)
; this `cardinality`'s value might be `-1` in some cases
is that ok to answer `true` if `cardinality` is `-1` ?
note: if `cardinality == -1` ; then both the old and the new logic returns
`true` - is that ok?
--
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]
