Skip to content

Metadata/dictionary based aggregation ignores column-type preconditions for MINLONG/MAXLONG and MINSTRING/MAXSTRING #19145

Description

@yashmayya

Summary

The non-scan (metadata/dictionary based) aggregation path treats an aggregation as resolvable if the column merely has a dictionary, without checking that the column type is one the function actually supports. For the typed MIN/MAX variants this produces two failures in opposite directions:

  • MINLONG / MAXLONG over FLOAT / DOUBLE / BIG_DECIMAL — the resolver is stricter than the eligibility check, so the query fails outright with IllegalArgumentException, even though the scan path computes it fine.
  • MINSTRING / MAXSTRING over a numeric column — the resolver is looser, so it silently returns the raw dictionary min/max (a boxed number) for a query that Min/MaxStringAggregationFunction explicitly reject as invalid. The value returned isn't MINSTRING/MAXSTRING semantics either: these are lexicographic, so over values 1..10 the max would be "9", not the 10 that comes back from the dictionary.

Current behaviour on master (verified at 9d6545bde1), for a table with dictionary-encoded doubleCol = i + 0.5 and intCol = i, i = 1..10:

Query Operator Result
SELECT MINLONG(doubleCol) NonScanBasedAggregationOperator IllegalArgumentException: MINLONG aggregation function can only be applied to columns of integer types
SELECT MAXLONG(doubleCol) NonScanBasedAggregationOperator IllegalArgumentException
SELECT MINLONG(doubleCol), SUM(intCol) AggregationOperator (partial) IllegalArgumentException
SELECT MINSTRING(intCol) NonScanBasedAggregationOperator ⚠️ silently returns 1 (Integer)
SELECT MAXSTRING(intCol) NonScanBasedAggregationOperator ⚠️ silently returns 10 (Integer)
SELECT MINSTRING(intCol), SUM(intCol) AggregationOperator (partial) ⚠️ silently returns 1 (Integer)

Root cause

AggregationPlanNode#isFitForNonScanBasedPlan treats any dictionary-encoded column as resolvable for the functions in DICTIONARY_BASED_FUNCTIONS:

if (dataSource.getDictionary() != null && DICTIONARY_BASED_FUNCTIONS.contains(functionType)) {
  return true;   // <- no column-type check
}

That set includes MINLONG, MAXLONG, MINSTRING and MAXSTRING (L50-L56), but the resolvers in AggregationFunctionUtils#getAggregationResult have different requirements:

There is already a guard of exactly the right shape immediately above, added by #18334 for MIN/MAX/MINMAXRANGE (L64-L68, L219-L223) — it just uses isNumeric(), which is not strict enough for MINLONG/MAXLONG (DOUBLE is numeric but not an integer type) and doesn't cover MINSTRING/MAXSTRING at all.

What changed with #18334

#18334 (merged as 9d6545b) extended metadata-based resolution to queries where only some aggregations are resolvable. That's a good change, but because the eligibility check doesn't encode these type preconditions, it also removed the workaround that previously masked this bug. Before that PR, adding any scanned aggregation forced the whole query onto the scan path; now the typed MIN/MAX is resolved from metadata regardless. Verified pre-merge at dc95530c8d vs post-merge at 9d6545bde1:

Query Before #18334 After #18334
SELECT MINLONG(doubleCol), SUM(intCol) ✅ returns 1 IllegalArgumentException
SELECT MINSTRING(intCol), SUM(intCol) BadQueryRequestException (correctly rejected) ⚠️ silently returns 1 (Integer)

So mixed queries went from working to failing in the first case, and from correctly rejected to silently wrong in the second. Single-aggregation queries behaved this way before #18334 as well — that part is long-standing, not a regression.

Expected behaviour

A query shouldn't change its answer, or flip between succeeding and failing, based on whether it qualifies for the non-scan plan:

  • MINLONG/MAXLONG over a non-integer numeric column should return the same value the scan path returns.
  • MINSTRING/MAXSTRING over a numeric column should be rejected with the same BadQueryRequestException the scan path raises, not answered from the dictionary.

Suggested fix

Make the eligibility check encode the resolvers' real type preconditions, so unsupported combinations fall back to the scan path (which already yields the correct value or the correct error):

  • MINLONG / MAXLONG: require stored type INT or LONG.
  • MINSTRING / MAXSTRING: require stored type STRING.

Reproduction

SELECT MINLONG(doubleCol) FROM myTable;
SELECT MAXLONG(doubleCol) FROM myTable;
SELECT MINLONG(doubleCol), SUM(intCol) FROM myTable;
SELECT MINSTRING(intCol) FROM myTable;
SELECT MAXSTRING(intCol) FROM myTable;
SELECT MINSTRING(intCol), SUM(intCol) FROM myTable;

Offline table, 10 rows, two dictionary-encoded metric columns: doubleCol = i + 0.5, intCol = i for i = 1..10. Run through InstancePlanMakerImplV2 against a generated segment.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething is not working as expecteddata-integrityRelated to correctness of data or query resultsqueryRelated to query processing

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions