Skip to content

feat: Union types coercion - #3513

Merged
alamb merged 1 commit into
apache:masterfrom
cube-js:union-types-coercion
Sep 19, 2022
Merged

feat: Union types coercion#3513
alamb merged 1 commit into
apache:masterfrom
cube-js:union-types-coercion

Conversation

@gandronchik

@gandronchikgandronchik commented Sep 16, 2022

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Union columns with different types

closes#3467

@github-actionsgithub-actionsBot added core Core DataFusion crate logical-expr Logical plan and expressions sql SQL Planner labels Sep 16, 2022
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #3513 (57962b9) into master (25c029c) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## master #3513 +/- ##
==========================================
- Coverage 85.75% 85.74% -0.01% 
==========================================
Files 299 299 Lines 55311 55352 +41 ==========================================
+ Hits 47432 47462 +30 - Misses 7879 7890 +11 
Impacted FilesCoverage Δ
datafusion/core/src/physical_plan/union.rs96.93% <100.00%> (+0.27%)⬆️
datafusion/expr/src/expr_rewriter.rs81.18% <100.00%> (+0.64%)⬆️
datafusion/expr/src/logical_plan/builder.rs90.37% <100.00%> (+0.20%)⬆️
datafusion/sql/src/planner.rs81.07% <100.00%> (+0.10%)⬆️
datafusion/common/src/dfschema.rs93.63% <0.00%> (-4.15%)⬇️
datafusion/expr/src/logical_plan/plan.rs77.68% <0.00%> (-0.17%)⬇️
datafusion/common/src/scalar.rs85.18% <0.00%> (+0.06%)⬆️
datafusion/core/src/physical_plan/metrics/value.rs87.43% <0.00%> (+0.50%)⬆️
datafusion/expr/src/expr_schema.rs63.47% <0.00%> (+0.59%)⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@alambalamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @gandronchik -- I think this PR is looking really good.

The only thing I think is needed prior to merge is 2 more tests. ❤️ Thank you!

Comment threaddatafusion/expr/src/expr_rewriter.rs
Comment threaddatafusion/expr/src/logical_plan/builder.rs Outdated
Comment threaddatafusion/sql/src/planner.rs
@gandronchik

Copy link
Copy Markdown
ContributorAuthor

Thanks @gandronchik -- I think this PR is looking really good.

The only thing I think is needed prior to merge is 2 more tests. ❤️ Thank you!

Hi @alamb. Thank you for review. I added more tests and also fixed few bugs 🙏

@alambalamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks great -- thank you @gandronchik . I went over the tests carefully and 👍

@alamb
alamb merged commit 4b1e044 into apache:masterSep 19, 2022
@ursabot

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = f30fc4e and contender = 4b1e044. 4b1e044 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

gandronchik added a commit to cube-js/arrow-datafusion that referenced this pull request Sep 20, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coreCore DataFusion cratelogical-exprLogical plan and expressionssqlSQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Union columns with different types

4 participants

@gandronchik@codecov-commenter@ursabot@alamb