Support casting `NULL` to/from `Decimal` #1922

liukun4515 · 2022-06-22T12:37:36Z

Which issue does this PR close?

Closes #1921

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

liukun4515 · 2022-06-22T12:38:49Z

@viirya @alamb @tustvold PTAL

alamb

Thanks @liukun4515 -- looks good to me

alamb · 2022-06-22T13:13:28Z

arrow/src/compute/kernels/cast.rs

Why not support casting Decimal to Null? If we don't add support in this PR can / should file a ticket to add it in a follow on I think

I will fill up Decimal to Null in this pull request.

alamb · 2022-06-22T13:14:16Z

Looks like there is a test failure: https://github.com/apache/arrow-rs/runs/7004119046?check_suite_focus=true

liukun4515 · 2022-06-22T13:43:36Z

Looks like there is a test failure: https://github.com/apache/arrow-rs/runs/7004119046?check_suite_focus=true

I will try to fix this case.

liukun4515 · 2022-06-22T15:46:15Z

IT failure: https://github.com/apache/arrow-rs/runs/7006919197?check_suite_focus=true#step:6:12919

Testing file auth:basic_proto
==========================================================
Traceback (most recent call last):
  File "/arrow/dev/archery/archery/integration/runner.py", line 246, in _run_ipc_test_case
    run_binaries(producer, consumer, test_case)
  File "/arrow/dev/archery/archery/integration/runner.py", line 286, in _produce_consume
    consumer.stream_to_file(producer_stream_path, consumer_file_path)
  File "/arrow/dev/archery/archery/integration/tester_csharp.py", line 63, in stream_to_file
    self.run_shell_command(cmd)
  File "/arrow/dev/archery/archery/integration/tester.py", line 49, in run_shell_command
    subprocess.check_call(cmd, shell=True)
  File "/opt/conda/envs/arrow/lib/python3.10/subprocess.py", line 369, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '/arrow/csharp/artifacts/Apache.Arrow.IntegrationTest/Debug/net6.0/Apache.Arrow.IntegrationTest --mode stream-to-file -a /tmp/tmpawjt5bid/9e0ef030_generated_datetime.consumer_stream_as_file < /tmp/tmpawjt5bid/9e0ef030_generated_datetime.producer_file_as_stream' returned non-zero exit status 1.
################# FAILURES #################
FAILED TEST: datetime Go producing,  C# consuming
1 failures
1

It has nothing about the arrow-rs

arrow/src/compute/kernels/cast.rs

liukun4515 · 2022-06-23T02:36:41Z

Please hold this pr and don't merge it.
I think we need to discuss some rules about casting.
@alamb @viirya

liukun4515 · 2022-06-24T04:08:08Z

Do we need to merge this into the release 17.0.0？
@alamb @viirya
In the follow-up pr of apache/datafusion#2764 we need the casting Null to Decimal Data type.

…-rs into support_NULL_to_decimal

viirya · 2022-06-24T05:30:43Z

Sounds good to merge this to 17.0.0.

liukun4515 · 2022-06-24T05:35:38Z

Sounds good to merge this to 17.0.0.

Thanks @viirya
If the ci pass, please help to merge it.

codecov-commenter · 2022-06-24T05:47:54Z

Codecov Report

Merging #1922 (3a7344a) into master (50a9dd5) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1922      +/-   ##
==========================================
+ Coverage   83.42%   83.47%   +0.05%     
==========================================
  Files         214      221       +7     
  Lines       57004    57042      +38     
==========================================
+ Hits        47553    47615      +62     
+ Misses       9451     9427      -24

Impacted Files	Coverage Δ
arrow/src/compute/kernels/cast.rs	`95.80% <100.00%> (+0.03%)`	⬆️
arrow/src/array/builder/buffer_builder.rs	`85.49% <0.00%> (-7.31%)`	⬇️
parquet/src/arrow/array_reader/mod.rs	`88.23% <0.00%> (-2.54%)`	⬇️
arrow/src/datatypes/datatype.rs	`65.42% <0.00%> (-0.38%)`	⬇️
arrow/src/array/data.rs	`84.16% <0.00%> (ø)`
parquet/src/arrow/arrow_writer/mod.rs	`97.53% <0.00%> (ø)`
parquet/src/arrow/array_reader/primitive_array.rs	`89.65% <0.00%> (ø)`
...uet/src/arrow/array_reader/complex_object_array.rs	`94.27% <0.00%> (ø)`
parquet/src/arrow/array_reader/struct_array.rs	`88.67% <0.00%> (ø)`
parquet/src/arrow/array_reader/null_array.rs	`88.00% <0.00%> (ø)`
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50a9dd5...3a7344a. Read the comment docs.

liukun4515 · 2022-06-27T09:05:12Z

thank you all!!

* support NULL type values to decimal * support NULL type values to decimal * Update arrow/src/compute/kernels/cast.rs * Update arrow/src/compute/kernels/cast.rs Co-authored-by: Liang-Chi Hsieh <[email protected]>

Can drop this after rebase on commit 9e8c1bf "Support casting NULL to/from Decimal (apache#1922)", first released in 17.0.0

github-actions bot added the arrow Changes to the arrow crate label Jun 22, 2022

liukun4515 marked this pull request as ready for review June 22, 2022 12:37

alamb changed the title ~~support NULL type values to decimal~~ Support casting NULL to/from Decimal Jun 22, 2022

alamb approved these changes Jun 22, 2022

View reviewed changes

liukun4515 force-pushed the support_NULL_to_decimal branch 2 times, most recently from 9692a39 to b89c268 Compare June 22, 2022 15:04

support NULL type values to decimal

369b203

liukun4515 force-pushed the support_NULL_to_decimal branch from b89c268 to 369b203 Compare June 22, 2022 15:46

viirya reviewed Jun 22, 2022

View reviewed changes

arrow/src/compute/kernels/cast.rs Outdated Show resolved Hide resolved

viirya reviewed Jun 22, 2022

View reviewed changes

arrow/src/compute/kernels/cast.rs Outdated Show resolved Hide resolved

support NULL type values to decimal

5bcfde9

liukun4515 requested a review from alamb June 23, 2022 03:26

liukun4515 mentioned this pull request Jun 24, 2022

Disallow cast from other datatypes to NullType #1923

Closed

viirya added 2 commits June 23, 2022 22:21

Update arrow/src/compute/kernels/cast.rs

fad7452

Update arrow/src/compute/kernels/cast.rs

46d022d

viirya approved these changes Jun 24, 2022

View reviewed changes

Merge branch 'support_NULL_to_decimal' of github.com:liukun4515/arrow…

3a7344a

…-rs into support_NULL_to_decimal

viirya merged commit 9e8c1bf into apache:master Jun 24, 2022

liukun4515 deleted the support_NULL_to_decimal branch June 27, 2022 09:05

mcheshkov pushed a commit to cube-js/arrow-rs that referenced this pull request Aug 21, 2024

feat: Support cast between Decimal and Null

796bfcc

Can drop this after rebase on commit 9e8c1bf "Support casting NULL to/from Decimal (apache#1922)", first released in 17.0.0

Support casting NULL to/from Decimal #1922

Support casting NULL to/from Decimal #1922

Uh oh!

Conversation

liukun4515 commented Jun 22, 2022

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

liukun4515 commented Jun 22, 2022

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jun 22, 2022

Choose a reason for hiding this comment

Uh oh!

liukun4515 Jun 22, 2022

Choose a reason for hiding this comment

Uh oh!

alamb commented Jun 22, 2022

Uh oh!

liukun4515 commented Jun 22, 2022

Uh oh!

liukun4515 commented Jun 22, 2022

Uh oh!

Uh oh!

Uh oh!

liukun4515 commented Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liukun4515 commented Jun 24, 2022

Uh oh!

viirya commented Jun 24, 2022

Uh oh!

liukun4515 commented Jun 24, 2022

Uh oh!

codecov-commenter commented Jun 24, 2022

Codecov Report

Uh oh!

liukun4515 commented Jun 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Support casting `NULL` to/from `Decimal` #1922

Support casting `NULL` to/from `Decimal` #1922

liukun4515 commented Jun 23, 2022 •

edited

Loading