)]}'
{
  "commit": "07e63edfa01079acf1c7571b7be914edfe324b02",
  "tree": "cde9d3d85a9346bbe826e6320f3247297dcee246",
  "parents": [
    "b7091c0d2f7ee915bd6d1d213bc2a27d8802ae63"
  ],
  "author": {
    "name": "kosiew",
    "email": "kosiew@gmail.com",
    "time": "Fri Jan 09 21:27:31 2026 +0800"
  },
  "committer": {
    "name": "GitHub",
    "email": "noreply@github.com",
    "time": "Fri Jan 09 13:27:31 2026 +0000"
  },
  "message": "Fix TopK aggregation for UTF-8/Utf8View group keys and add safe fallback for unsupported string aggregates (#19285)\n\n## Which issue does this PR close?\n\n* Closes #19219.\n\n## Rationale for this change\n\nA `GROUP BY ... ORDER BY \u003caggregate\u003e ... LIMIT` query can trigger\nDataFusion’s TopK aggregation optimization. In affected releases,\nqueries grouping by text columns—especially `Utf8View` produced via SQL\n`varchar` mappings / `arrow_cast`—could fail at execution time with an\nerror such as `Can\u0027t group type: Utf8View`.\n\nThis happens because the optimizer may select the TopK aggregation path\neven when the underlying TopK data structures (heap/hash table) do not\nfully support the specific key/value Arrow types involved. Disabling\n`datafusion.optimizer.enable_topk_aggregation` is a workaround, but it\nforces users to trade correctness for performance.\n\nThis PR makes TopK type support explicit and consistent across the\noptimizer and execution, adds support for UTF-8 string value heaps, and\nensures unsupported key/value combinations fall back to the standard\naggregation implementation rather than panicking.\n\n## What changes are included in this PR?\n\n* **Centralized TopK type validation**\n\n* Introduced `topk_types_supported(key_type, value_type)` (in\n`physical-plan/src/aggregates/mod.rs`) to validate both grouping key and\nmin/max value types.\n* Optimizer now uses this shared check rather than duplicating partial\ntype logic.\n\n* **Safer AggregateExec cloning for limit pushdown**\n\n* Added `AggregateExec::with_new_limit` to clone an aggregate exec while\noverriding only the TopK `limit` hint, avoiding manual reconstruction\nand ensuring plan properties/fields remain consistent.\n\n* **TopK hash table improvements + helper functions**\n\n* Added `is_supported_hash_key_type` helper for grouping key\ncompatibility checks.\n  * Refactored string key extraction to a single helper function.\n* Added `find_or_insert` entry API to avoid double lookups and unify\ninsertion behavior.\n\n* **TopK heap support for string aggregate values**\n\n* Added `StringHeap` implementation supporting `Utf8`, `LargeUtf8`, and\n`Utf8View` aggregate values using lexicographic ordering.\n* Added `is_supported_heap_type` helper for aggregate value\ncompatibility.\n* Updated `new_heap` to create `StringHeap` for supported string types\nand return a clearer error message for unsupported types.\n\n* **Debug contract in TopK stream**\n\n* Added a debug assertion in `GroupedTopKAggregateStream` documenting\nthat type validation should have already happened (optimizer +\ncan_use_topk), without affecting release builds.\n\n## Are these changes tested?\n\nYes.\n\n* Added a new physical optimizer test covering UTF-8 grouping with:\n\n1. **Supported** numeric `max/min` value (TopK should be used and\nresults correct)\n2. **Unsupported** string `max/min` value (must fall back to standard\naggregation and not use `GroupedTopKAggregateStream`)\n\n* Added unit tests in `PriorityMap` to validate lexicographic `min/max`\ntracking for:\n\n  * `Utf8`\n  * `LargeUtf8`\n  * `Utf8View`\n\n* Added SQLLogicTest coverage (`aggregates_topk.slt`) for:\n\n  * `varchar` tables\n  * `Utf8View` via `arrow_cast`\n* `EXPLAIN` verification that TopK limit propagation is applied and\nplans remain stable\n  * Regression case for `max(trace_id)` with `ORDER BY ... LIMIT`\n\n## Are there any user-facing changes?\n\nYes (bug fix).\n\n* Queries that group by text columns (including `Utf8View`) and use\n`ORDER BY \u003caggregate\u003e ... LIMIT` should no longer error.\n* TopK aggregation now supports UTF-8 string aggregate values for\nmin/max (lexicographic ordering) where applicable.\n* For unsupported type combinations, DataFusion will fall back\ngracefully to the standard aggregation path instead of panicking.\n\nNo breaking public API changes are intended. The only new public helper\nAPIs are internal to the physical plan modules.\n\n## LLM-generated code disclosure\n\nThis PR includes LLM-generated code and comments. All LLM-generated\ncontent has been manually reviewed and tested.",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "be193f873713bbd7a85be5af44264b5e092accaa",
      "old_mode": 33188,
      "old_path": "datafusion/core/benches/topk_aggregate.rs",
      "new_id": "7979efdec605e63df633bc2974c0023351a5cee7",
      "new_mode": 33188,
      "new_path": "datafusion/core/benches/topk_aggregate.rs"
    },
    {
      "type": "modify",
      "old_id": "1fdc0ae6c7f60a6e1ed988dc5d832c75a8a981a1",
      "old_mode": 33188,
      "old_path": "datafusion/core/tests/physical_optimizer/aggregate_statistics.rs",
      "new_id": "4218f76fa135ae1d8918c5752bfb49b709d43338",
      "new_mode": 33188,
      "new_path": "datafusion/core/tests/physical_optimizer/aggregate_statistics.rs"
    },
    {
      "type": "modify",
      "old_id": "7eb9e6a76211b48449e9d2c5feb0ed92e94cd3c7",
      "old_mode": 33188,
      "old_path": "datafusion/physical-optimizer/src/topk_aggregation.rs",
      "new_id": "7b2983ee7199629c9155bc0dfcca7263f9eaaf81",
      "new_mode": 33188,
      "new_path": "datafusion/physical-optimizer/src/topk_aggregation.rs"
    },
    {
      "type": "modify",
      "old_id": "b0828ff232f94e6ec428a8deb6bb3ba62f21f76a",
      "old_mode": 33188,
      "old_path": "datafusion/physical-plan/src/aggregates/mod.rs",
      "new_id": "4dd9482ac43226c417391c0c2c4d603df62acdf6",
      "new_mode": 33188,
      "new_path": "datafusion/physical-plan/src/aggregates/mod.rs"
    },
    {
      "type": "modify",
      "old_id": "4a3f3ac258f9edae4dd8d166e937b5e605ca4c06",
      "old_mode": 33188,
      "old_path": "datafusion/physical-plan/src/aggregates/topk/hash_table.rs",
      "new_id": "418ec49ddd718e87bbb286fcd4c3742aeb9f33ac",
      "new_mode": 33188,
      "new_path": "datafusion/physical-plan/src/aggregates/topk/hash_table.rs"
    },
    {
      "type": "modify",
      "old_id": "b4569c3d0811d77b8da1bae205f0616250464baf",
      "old_mode": 33188,
      "old_path": "datafusion/physical-plan/src/aggregates/topk/heap.rs",
      "new_id": "9f0b697ccabeeb53f5a5ac78d57676e69aef0eff",
      "new_mode": 33188,
      "new_path": "datafusion/physical-plan/src/aggregates/topk/heap.rs"
    },
    {
      "type": "modify",
      "old_id": "8e093d213e784041218afe51933ff12133bdf2e1",
      "old_mode": 33188,
      "old_path": "datafusion/physical-plan/src/aggregates/topk/priority_map.rs",
      "new_id": "c74b648d373ceb689d69d3722f69305855fb3ee9",
      "new_mode": 33188,
      "new_path": "datafusion/physical-plan/src/aggregates/topk/priority_map.rs"
    },
    {
      "type": "modify",
      "old_id": "1096eb64d3ae7d3eec3fe691737779f88a70750b",
      "old_mode": 33188,
      "old_path": "datafusion/physical-plan/src/aggregates/topk_stream.rs",
      "new_id": "a43b5cff1298943c613a950e8b119650c7e2089a",
      "new_mode": 33188,
      "new_path": "datafusion/physical-plan/src/aggregates/topk_stream.rs"
    },
    {
      "type": "modify",
      "old_id": "58abecfacfa8d03be5ad196610b3351da18869d0",
      "old_mode": 33188,
      "old_path": "datafusion/sqllogictest/test_files/aggregates_topk.slt",
      "new_id": "05f3e02bbc1b37506de90111463eaf5ec9b803a8",
      "new_mode": 33188,
      "new_path": "datafusion/sqllogictest/test_files/aggregates_topk.slt"
    }
  ]
}