Group By Truncates at Limit in Combine Operator

[ankitsultana]

Queries such as the following return incorrect results in most cases because the Combine Operator truncates the IndexedTable at the Query Limit. Moreover there are no warnings in the response:

SELECT COUNT(*), cityID
FROM my_table
GROUP BY cityID
LIMIT 10

Relevant code for this is here:

pinot/pinot-core/src/main/java/org/apache/pinot/core/util/GroupByUtils.java

Lines 110 to 125 in 7293122

	// When there is no ORDER BY, trim is not required because the indexed table stops accepting new groups once the
	// result size is reached
	if (!hasOrderBy) {
	int resultSize;
	if (hasHaving) {
	// Keep more groups when there is HAVING clause
	resultSize = trimSize;
	} else {
	// TODO: Keeping only 'LIMIT' groups can cause inaccurate result because the groups are randomly selected
	// without ordering. Consider ordering on group-by columns if no ordering is specified.
	resultSize = limit;
	}
	int initialCapacity = getIndexedTableInitialCapacity(resultSize, numGroups, minInitialIndexedTableCapacity);
	return getTrimDisabledIndexedTable(dataSchema, false, queryContext, resultSize, initialCapacity, numThreads,
	executorService);
	}

I think we can do a couple of things to improve this:

1. Add a server level config to control the trim for Group By without Order-By.
2. Set a flag in the response to indicate a trim was done at server level.

Marking this as a bug because the documentation for Grouping Algorithm does not call this out explicitly: https://docs.pinot.apache.org/users/user-guide-query/query-syntax/grouping-algorithm

[Jackie-Jiang]

This is tracked under #11924

[Jackie-Jiang]

Duplicate of #11706

[anandkrshaw]

@ankitsultana @Jackie-Jiang
An implicit order by will not harm. right ?
That looks to be an cleaner and consistent approach.

[ankitsultana]

It will help a bit but from our perspective the main requirement is to return a warning to indicate that results are partial.

There are some other scenarios of partial results that we want to handle too and one of our engineers will take a stab at it in the next month or two.