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

+        }
+
+        // Now: can't convert decimal to NULL type
+        let array = new_null_array(&data_type, 4);


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/arrow-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 <viirya@gmail.com>

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

liukun4515 commented Jun 22, 2022

liukun4515 commented Jun 22, 2022

alamb left a comment

alamb Jun 22, 2022

liukun4515 Jun 22, 2022

alamb commented Jun 22, 2022

liukun4515 commented Jun 22, 2022

liukun4515 commented Jun 22, 2022

liukun4515 commented Jun 23, 2022 •

edited

liukun4515 commented Jun 24, 2022

viirya commented Jun 24, 2022

liukun4515 commented Jun 24, 2022

codecov-commenter commented Jun 24, 2022

liukun4515 commented Jun 27, 2022

Support casting NULL to/from Decimal #1922

Support casting NULL to/from Decimal #1922

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?

liukun4515 commented Jun 22, 2022

alamb left a comment

Choose a reason for hiding this comment

alamb Jun 22, 2022

Choose a reason for hiding this comment

liukun4515 Jun 22, 2022

Choose a reason for hiding this comment

alamb commented Jun 22, 2022

liukun4515 commented Jun 22, 2022

liukun4515 commented Jun 22, 2022

liukun4515 commented Jun 23, 2022 • edited

liukun4515 commented Jun 24, 2022

viirya commented Jun 24, 2022

liukun4515 commented Jun 24, 2022

codecov-commenter commented Jun 24, 2022

Codecov Report

liukun4515 commented Jun 27, 2022

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

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

liukun4515 commented Jun 23, 2022 •

edited