Skip to content

Conversation

@findepi
Copy link
Member

@findepi findepi commented Aug 13, 2025

@github-actions github-actions bot added the logical-expr Logical plan and expressions label Aug 13, 2025
@findepi findepi force-pushed the findepi/deprecate-unused-scalarudf-display-name-2bac58 branch from 0e0fbec to a31a0f8 Compare August 13, 2025 10:36
@findepi findepi requested review from alamb and jayzhan211 August 13, 2025 11:49
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.

Makes sense to me

Looks like it was added by @yyy1000 in

cc @jayzhan211

There are a bunch of implementations in the DataFusion function library, as you can see here: https://github.com/search?q=repo%3Aapache%2Fdatafusion%20display_name&type=code

However I didn't see any uses either

@findepi
Copy link
Member Author

findepi commented Aug 13, 2025

Maybe the "usage" is here, in a TODO comment

// write!(f, "{}", func.display_name(args).unwrap())

We need to decide what to do.
Keep display_name() code that's dead/redundant for a year now.
Or proceed with removal. Then the above comment should be gone.

As #11782, #10364 (comment) note, having many "name" functions is hard to follow, so I'm inclined to proceed with removal.

@alamb
Copy link
Contributor

alamb commented Aug 13, 2025

Or proceed with removal. Then the above comment should be gone.

I think we should remove it

maybe since the deprecation notice doesn't actually generate any warnings, we should just remove the function (with a breaking API change) immediately 🤔

@findepi
Copy link
Member Author

findepi commented Aug 13, 2025

right, for folks implementing ScalarUDFImpl it's a breakage sooner or later
for folks calling ScalarUDFImpl::display_name or ScalarUDF::display_name, it's a graceful deprecation with a warning, so still a bonus

@findepi
Copy link
Member Author

findepi commented Aug 13, 2025

Or proceed with removal. Then the above comment should be gone.

I think we should remove it

Coment is gone.

@findepi findepi merged commit 71408cf into apache:main Aug 14, 2025
27 checks passed
@findepi findepi deleted the findepi/deprecate-unused-scalarudf-display-name-2bac58 branch August 14, 2025 10:40
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