Skip to content

fix: preserve argument types in max_by/min_by with dictionary inputs#15

Merged
dariocurr merged 2 commits into
datafusion-contrib:mainfrom
ahmed-mez:ahmed/min-max-dict
Oct 21, 2025
Merged

fix: preserve argument types in max_by/min_by with dictionary inputs#15
dariocurr merged 2 commits into
datafusion-contrib:mainfrom
ahmed-mez:ahmed/min-max-dict

Conversation

@ahmed-mez
Copy link
Copy Markdown
Contributor

Summary

This PR fixes a type inference issue in max_by and min_by aggregate functions when the first argument is a dictionary type.

Problem

Previously, when max_by or min_by received a dictionary as the first argument, the get_min_max_by_result_type function would only return the dictionary's value type, discarding information about other arguments. This could lead to type mismatches in query planning when the function result was used in subsequent operations.

Error during planning: Failed to coerce arguments to satisfy a call to 'max_by' function: coercion from [Dictionary(UInt32, Utf8), Int64] to the signature UserDefined failed No function matches the given name and argument types 'max_by(Dictionary(UInt32, Utf8), Int64)'

Solution

The fix modifies get_min_max_by_result_type to:

  • Extract the dictionary's value type for the first argument (as before)
  • Preserve all other argument types in the result vector
  • Return the complete type information for proper query planning

Testing

  • max_by with string/int, string/float, float/string, int/string combinations
  • min_by with the same type combinations
  • Both functions with dictionary-typed first arguments
  • Validates correct result values and types

All tests pass successfully.

@dariocurr dariocurr self-requested a review October 17, 2025 08:22
@ahmed-mez
Copy link
Copy Markdown
Contributor Author

Hey @dariocurr 👋 could you please take a look when you get a chance? Thanks!

@dariocurr dariocurr changed the base branch from main to min-max-dict October 21, 2025 12:14
@dariocurr dariocurr changed the base branch from min-max-dict to main October 21, 2025 12:15
@dariocurr
Copy link
Copy Markdown
Collaborator

Hey @dariocurr 👋 could you please take a look when you get a chance? Thanks!

Hi @ahmed-mez!
Thank you for your time and contribution.
I merged your branch in #19 in order to clean up a bit the code.
I'll wait for your confirmation before merging

@dariocurr dariocurr merged commit 32767c3 into datafusion-contrib:main Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants