xiangfu0 commented on code in PR #18725:
URL: https://github.com/apache/pinot/pull/18725#discussion_r3388168553
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/InstanceResponseOperator.java:
##########
@@ -96,6 +97,13 @@ protected InstanceResponseBlock
buildInstanceResponseBlock(BaseResultsBlock base
String.valueOf(_threadMemAllocatedBytes));
instanceResponseBlock.addMetadata(MetadataKey.SYSTEM_ACTIVITIES_CPU_TIME_NS.getName(),
String.valueOf(_systemActivitiesCpuTimeNs));
+ Integer implicitLimit =
QueryOptionsUtils.getLiteModeImplicitLeafStageLimit(
+ _queryContext.getQueryOptions());
+ // false-positive when table has exactly implicitLimit rows
+ if (implicitLimit != null && baseResultsBlock.getNumRows() >=
implicitLimit) {
Review Comment:
`numRows >= implicitLimit` is not a valid truncation signal. When the
complete result cardinality is exactly the implicit limit, this marks
`mseLiteLeafStageLimitReached=true` and `partialResult=true` even though
nothing was dropped; the new equality-case test bakes in that false positive.
This needs an explicit "limit actually hit" signal from the limiting operator
(and the same fix is needed in `StreamingInstanceResponseOperator`) instead of
inferring truncation from equality.
--
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]
