Skip to content

fix: core_expressions feature flag broken, move overlay into core functions - #15217

Merged
alamb merged 6 commits into
apache:mainfrom
shruti2522:overlay-err
Mar 24, 2025
Merged

fix: core_expressions feature flag broken, move overlay into core functions#15217
alamb merged 6 commits into
apache:mainfrom
shruti2522:overlay-err

Conversation

@shruti2522

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

deprecate core_expressions fetaure flag and move overlay to core

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions Bot added the functions Changes to functions implementation label Mar 14, 2025
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Mar 14, 2025
@alamb alamb changed the title fix: core_expressions feature flag broken fix: core_expressions feature flag broken,, move coverlay into core functions Mar 14, 2025

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you @shruti2522 🙏

The only concern I have about this PR is that now overlay() is not listed as a string function, which is kind of strange.

It also means that anyone who usesoverlay in their project would have to change the use path.

I wonder if it would be possible to also have it listed with the string functions (as well as core)) maybe by re-exporting it in datafusion-functions/string somehow 🤔

@shruti2522

Copy link
Copy Markdown
Contributor Author

Thank you @shruti2522 🙏

The only concern I have about this PR is that now overlay() is not listed as a string function, which is kind of strange.

It also means that anyone who usesoverlay in their project would have to change the use path.

I wonder if it would be possible to also have it listed with the string functions (as well as core)) maybe by re-exporting it in datafusion-functions/string somehow 🤔

hey @alamb, I have already added a re-export at the end of datafusion/functions/src/string/overlay.rs like this

pub use crate::core::overlay::*;

Maybe, I should remove the deprecation statement for overlay() in string.rs. Should we consider a different approach for this?

@alamb alamb changed the title fix: core_expressions feature flag broken,, move coverlay into core functions fix: core_expressions feature flag broken, move overlay into core functions Mar 16, 2025
@alamb

alamb commented Mar 18, 2025

Copy link
Copy Markdown
Contributor

hey @alamb, I have already added a re-export at the end of datafusion/functions/src/string/overlay.rs like this

Thanks @shruti2522 - that looks good to me

I double checked and it is appearing as we would expect

Screenshot 2025-03-18 at 3 43 22 PM

Thank you 🙏

@shruti2522

Copy link
Copy Markdown
Contributor Author

hey @alamb, I have already added a re-export at the end of datafusion/functions/src/string/overlay.rs like this

Thanks @shruti2522 - that looks good to me

I double checked and it is appearing as we would expect

Screenshot 2025-03-18 at 3 43 22 PM

Thank you 🙏

Do I need to make any further changes in this PR @alamb ?

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry @shruti2522 -- I think this one looks good to me. I think I forgot to click approve

@github-actions github-actions Bot added the development-process Related to development process of DataFusion label Mar 24, 2025
@alamb

alamb commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

I merged up from main and removed a reference in the CI tests to the now removed core_expressions feature

@alamb
alamb merged commit 45ed5aa into apache:main Mar 24, 2025
@alamb

alamb commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

Thanks again @shruti2522

qstommyshu pushed a commit to qstommyshu/datafusion that referenced this pull request Mar 27, 2025
…e` functions (apache#15217)

* fix: remove core_expressions, move overlay to core

* fix license header

* fix header

* update doc

* Remove CI reference

---------

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
nirnayroy pushed a commit to nirnayroy/datafusion that referenced this pull request May 2, 2025
…e` functions (apache#15217)

* fix: remove core_expressions, move overlay to core

* fix license header

* fix header

* update doc

* Remove CI reference

---------

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

development-process Related to development process of DataFusion documentation Improvements or additions to documentation functions Changes to functions implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core_expressions feature is broken in the datafusion-functions

2 participants