Skip to content

Conversation

@findepi
Copy link
Member

@findepi findepi commented Aug 8, 2025

The aliased UDF impls -- AliasedScalarUDFImpl,
AliasedAggregateUDFImpl and AliasedWindowUDFImpl are supposed to forward all methods (except aliases to the underlying implementation. However, they failed to do so, with many optional methods not being forwarded.

This change fixes this for good. Adds the missing method implementations. Replaces a code comment with compile-time check that all methods are indeed implemented.

@github-actions github-actions bot added the logical-expr Logical plan and expressions label Aug 8, 2025
@findepi findepi requested review from alamb, lewiszlw and waynexia August 8, 2025 09:07
The aliased UDF impls -- `AliasedScalarUDFImpl`,
`AliasedAggregateUDFImpl` and `AliasedWindowUDFImpl` are supposed to
forward all methods (except `aliases` to the underlying implementation.
However, they failed to do so, with many optional methods not being
forwarded.

This change fixes this for good. Adds the missing method
implementations. Replaces a code comment with compile-time check that
all methods are indeed implemented.
@findepi findepi force-pushed the findepi/fill-missing-methods-in-aliased-udf-impls-971c00 branch from 96a8a6e to a03ca3f Compare August 8, 2025 09:20
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @findepi

}
}

#[warn(clippy::missing_trait_methods)] // Delegates, so it should implement every single trait method
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️
TIL missing_trait_methods 👍

@alamb alamb merged commit 9264bb8 into apache:main Aug 8, 2025
27 checks passed
@findepi findepi deleted the findepi/fill-missing-methods-in-aliased-udf-impls-971c00 branch August 8, 2025 20:21
shruti2522 pushed a commit to shruti2522/datafusion that referenced this pull request Aug 10, 2025
The aliased UDF impls -- `AliasedScalarUDFImpl`,
`AliasedAggregateUDFImpl` and `AliasedWindowUDFImpl` are supposed to
forward all methods (except `aliases` to the underlying implementation.
However, they failed to do so, with many optional methods not being
forwarded.

This change fixes this for good. Adds the missing method
implementations. Replaces a code comment with compile-time check that
all methods are indeed implemented.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

logical-expr Logical plan and expressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants