Skip to content

Remove redundantis_numeric and other checks for DataType #1613

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Closed
alamb opened this issue Jan 19, 2022 · 5 comments · Fixed by #7734
Closed

Remove redundantis_numeric and other checks for DataType #1613

alamb opened this issue Jan 19, 2022 · 5 comments · Fixed by #7734
Labels
good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Jan 19, 2022

arrow-rs has some functions to check data type, and the logic is also in datafusion, for example, https://docs.rs/arrow/7.0.0/arrow/datatypes/enum.DataType.html#method.is_numeric

We should reduce the redundancy in DataFusion to make the codebase clear.

Originally posted by @liukun4515 in #1606 (comment)
@alamb alamb mentioned this issue Jan 19, 2022
Merged
@liukun4515 liukun4515 mentioned this issue Jan 20, 2022
Merged
@alamb alamb added the good first issue Good for newcomers label Jan 21, 2022
@alamb
Copy link
Contributor Author

alamb commented Jan 21, 2022

This is a good one -- just delete some code from DataFusion 👍

@HaoYang670
Copy link
Contributor

is_numeric in arrow-rs seems not to be same as in arrow-datafusion because DataType::Decimal is not included.

@liukun4515
Copy link
Contributor

is_numeric in arrow-rs seems not to be same as in arrow-datafusion because DataType::Decimal is not included.

@HaoYang670

In the PG or Spark, the decimal is included in the numeric.
Because the decimal is in development, some codes may be not consistent.

@liukun4515
Copy link
Contributor

Now we can focus on other issues, and deal with them after other stabilization.

@HaoYang670 HaoYang670 mentioned this issue Aug 30, 2022
Closed
@HaoYang670
Copy link
Contributor

At least, should rename this function, so that developers would not misuse this and DataType::is_numeric.

qrilka added a commit to qrilka/arrow-datafusion that referenced this issue Oct 3, 2023
@qrilka
Remove redundant is_numeric for DataType
8a5e871 
It's available in arrow-rs since version 3.0.0

Resolves apache#1613
@qrilka qrilka mentioned this issue Oct 3, 2023
Merged
alamb pushed a commit that referenced this issue Oct 4, 2023
@qrilka
Remove redundant is_numeric for DataType (#7734)
91a8025 
It's available in arrow-rs since version 3.0.0

Resolves #1613
Ted-Jiang pushed a commit to Ted-Jiang/arrow-datafusion that referenced this issue Oct 7, 2023
@qrilka @Ted-Jiang
Remove redundant is_numeric for DataType (apache#7734)
1650fdd 
It's available in arrow-rs since version 3.0.0

Resolves apache#1613
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove redundant is_numeric for DataType qrilka/arrow-datafusion
3 participants
@alamb @liukun4515 @HaoYang670