Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor: Deprecate ScalarValue::raw_data #15016

Merged
merged 1 commit into from
Mar 6, 2025
Merged

Conversation

qazxcdswe123
Copy link
Contributor

@qazxcdswe123 qazxcdswe123 commented Mar 5, 2025

Which issue does this PR close?

  • Closes nothing

Rationale for this change

cleanup unused deadcode

What changes are included in this PR?

cleanup unused deadcode

Are these changes tested?

yes

Are there any user-facing changes?

no

I just realized this is a pub fn so if anyone is using this they may be affected. But this function does not seem to be useful?

@github-actions github-actions bot added the common Related to common crate label Mar 5, 2025
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 for the contirbution @qazxcdswe123

@@ -2764,15 +2764,6 @@ impl ScalarValue {
Ok(scalars)
}

// TODO: Support more types after other ScalarValue is wrapped with ArrayRef
/// Get raw data (inner array) inside ScalarValue
pub fn raw_data(&self) -> Result<ArrayRef> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you very much @qazxcdswe123 -- this is a nice find.

Per https://datafusion.apache.org/contributor-guide/api-health.html instead of removing this function, can you please mark it as #[deprecated] instead?

Maybe it would be a good time to go remove any deprecated functions from 41 or earlier 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the remind!

And I think to_array is what we meant to do here?

pub fn to_array(&self) -> Result<ArrayRef> {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll file another PR to cleanup some old deprecation

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.

Thank you @qazxcdswe123

@qazxcdswe123 qazxcdswe123 force-pushed the cleanup branch 2 times, most recently from b35e856 to 2fcc908 Compare March 5, 2025 14:27
@alamb alamb changed the title Minor: cleanup unused code Minor: Deprecate ScalarValue::raw_data Mar 5, 2025
@alamb alamb merged commit da42933 into apache:main Mar 6, 2025
25 checks passed
danila-b pushed a commit to danila-b/datafusion that referenced this pull request Mar 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Related to common crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants