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

Support CAST from Decimal datatype to String #3994

Merged
merged 6 commits into from
Apr 1, 2023
Merged

Conversation

comphead
Copy link
Contributor

Which issue does this PR close?

Closes #3991.

Rationale for this change

Support cast Decimal datatype to string

What changes are included in this PR?

Are there any user-facing changes?

No

@github-actions github-actions bot added the arrow Changes to the arrow crate label Mar 31, 2023
@comphead comphead changed the title Master Support CAST from Decimal datatype to String Mar 31, 2023
@comphead comphead marked this pull request as ready for review March 31, 2023 23:06
@comphead
Copy link
Contributor Author

@tustvold not sure why the test fails on

Test casting Decimal128(38, 0) --> LargeList(Field { name: "item", data_type: Utf8, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} })
thread 'test_can_cast_types' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidArgumentError("non-nullable child of type Utf8 contains nulls not present in parent LargeList(Field { name: \"item\", data_type: Utf8, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} })")', arrow-data/src/data/mod.rs:1795:30

Perhaps related to #3983

@tustvold
Copy link
Contributor

My guess is the cast is failing and inserting a null, but the field doesn't permit nulls. I'm not sure what the cast kernel should do in such a case tbh, but in the immediate term I'd suggest finding a way to avoid the decimal cast from failing

@comphead
Copy link
Contributor Author

comphead commented Apr 1, 2023

My guess is the cast is failing and inserting a null, but the field doesn't permit nulls. I'm not sure what the cast kernel should do in such a case tbh, but in the immediate term I'd suggest finding a way to avoid the decimal cast from failing

@tustvold I have removed None from decimal array as other tests casts with not nullable values

Copy link
Contributor

@tustvold tustvold left a comment

Choose a reason for hiding this comment

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

Just some minor nits

arrow-cast/src/cast.rs Outdated Show resolved Hide resolved
arrow-cast/src/cast.rs Show resolved Hide resolved
arrow-cast/src/cast.rs Outdated Show resolved Hide resolved
comphead and others added 3 commits April 1, 2023 09:50
Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>
@tustvold
Copy link
Contributor

tustvold commented Apr 1, 2023

I took the liberty of resolving the merge conflicts on this

@tustvold tustvold merged commit 591f0ef into apache:master Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Decimals cast to Utf8/LargeUtf
2 participants