Skip to content

Conversation

@plcplc
Copy link
Contributor

@plcplc plcplc commented Mar 22, 2024

What

This PR makes the introspection of aggregation functions take implicit casts into account.

This enables aggregation functions on domain types, which would typically only be defined on the base type.

It also enables certain other aggregations for scalar types that weren't defined before, most notably aggregation functions for float4 on CockroachDB, which becomes accessible through implicit casts to float8.

How

The cast-extension for aggregation functions introspection code is based on the same for comparison operators.

We also change our notion of "occurring scalar types" to include the return types aggregation functions. Previously, aggregation functions could be filtered out if their return type didn't feature in the set of occurring scalar types, which was unnecessarily strict.

@plcplc plcplc requested review from SamirTalwar and soupi March 22, 2024 20:43
@plcplc plcplc force-pushed the plc/cast-extend-aggregate-functions branch from 196aa62 to cd37a07 Compare March 22, 2024 20:46
@plcplc plcplc force-pushed the plc/cast-extend-aggregate-functions branch from cd37a07 to 99c364d Compare March 22, 2024 20:47
ORDER BY
-- Prefer uncast argument.
NOT argument_casted DESC,
-- Arbitrary desperation: Lexical ordering
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤣

Copy link
Contributor

@danieljharvey danieljharvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely great.

@plcplc plcplc enabled auto-merge March 25, 2024 10:39
@plcplc plcplc added this pull request to the merge queue Mar 25, 2024
Merged via the queue into main with commit 73ab921 Mar 25, 2024
@plcplc plcplc deleted the plc/cast-extend-aggregate-functions branch March 25, 2024 11:14
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