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

[SPARK-38018][SQL][3.2] Fix ColumnVectorUtils.populate to handle CalendarIntervalType correctly #37114

Closed
wants to merge 1 commit into from

Conversation

c21
Copy link
Contributor

@c21 c21 commented Jul 7, 2022

What changes were proposed in this pull request?

This is a backport of #35314 to branch 3.2. See that original PR for context.

Why are the changes needed?

To fix potential correctness issue.

Does this PR introduce any user-facing change?

No but fix the exiting correctness issue when reading partition column with CalendarInterval type.

How was this patch tested?

Added unit test in ColumnVectorSuite.scala.

@github-actions github-actions bot added the SQL label Jul 7, 2022
@c21
Copy link
Contributor Author

c21 commented Jul 7, 2022

Still waiting for the test results, but cc @cloud-fan and @dongjoon-hyun if you have time to take a look, thanks!

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

LGTM +1

@cloud-fan
Copy link
Contributor

tests all passed, but the reporting is broken: https://github.com/c21/spark/actions/runs/2627811518

cloud-fan pushed a commit that referenced this pull request Jul 7, 2022
…ndarIntervalType correctly

### What changes were proposed in this pull request?

This is a backport of #35314 to branch 3.2. See that original PR for context.

### Why are the changes needed?

To fix potential correctness issue.

### Does this PR introduce _any_ user-facing change?

No but fix the exiting correctness issue when reading partition column with CalendarInterval type.

### How was this patch tested?

Added unit test in `ColumnVectorSuite.scala`.

Closes #37114 from c21/branch-3.2.

Authored-by: Cheng Su <scnju13@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan
Copy link
Contributor

thanks, merging to 3.2

@cloud-fan cloud-fan closed this Jul 7, 2022
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @c21 , @LuciferYang , @cloud-fan .

@c21
Copy link
Contributor Author

c21 commented Jul 7, 2022

Thank you all for review!

@c21 c21 deleted the branch-3.2 branch July 7, 2022 22:45
sunchao pushed a commit to sunchao/spark that referenced this pull request Jun 2, 2023
…ndarIntervalType correctly

### What changes were proposed in this pull request?

This is a backport of apache#35314 to branch 3.2. See that original PR for context.

### Why are the changes needed?

To fix potential correctness issue.

### Does this PR introduce _any_ user-facing change?

No but fix the exiting correctness issue when reading partition column with CalendarInterval type.

### How was this patch tested?

Added unit test in `ColumnVectorSuite.scala`.

Closes apache#37114 from c21/branch-3.2.

Authored-by: Cheng Su <scnju13@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit c5983c1)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants