Skip to content

Conversation

@AngersZhuuuu
Copy link
Contributor

@AngersZhuuuu AngersZhuuuu commented Apr 22, 2021

What changes were proposed in this pull request?

IntervalUtils.fromYearMonthString should handle Int.MinValue months correctly.
In current logic, just use Math.addExact(Math.multiplyExact(years, 12), months) to calculate negative total months will overflow when actual total months is Int.MinValue, this pr fixes this bug.

Why are the changes needed?

IntervalUtils.fromYearMonthString should handle Int.MinValue months correctly

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added UT

@AngersZhuuuu
Copy link
Contributor Author

FYI @MaxGekk

@github-actions github-actions bot added the SQL label Apr 22, 2021
@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42283/

@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Test build #137756 has finished for PR 32281 at commit 20fdef3.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@sarutak
Copy link
Member

sarutak commented Apr 22, 2021

@AngersZhuuuu JIRA ID seems to be wrong. Could you confirm it?

@AngersZhuuuu AngersZhuuuu changed the title [SPARK-35117][SQL] IntervalUtils.fromYearMonthString should handle Int.MinValue months correctly [SPARK-35177][SQL] IntervalUtils.fromYearMonthString should handle Int.MinValue months correctly Apr 22, 2021
@AngersZhuuuu
Copy link
Contributor Author

@AngersZhuuuu JIRA ID seems to be wrong. Could you confirm it?

Done.

@AngersZhuuuu
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42293/

@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42293/

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

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

Could you make PR's title more precise, for example:
[SPARK-35177][SQL] Fix arithmetic overflow in parsing the minimal interval by IntervalUtils.fromYearMonthString

@AngersZhuuuu AngersZhuuuu changed the title [SPARK-35177][SQL] IntervalUtils.fromYearMonthString should handle Int.MinValue months correctly [SPARK-35177][SQL] Fix arithmetic overflow in parsing the minimal interval by IntervalUtils.fromYearMonthString Apr 22, 2021
@AngersZhuuuu
Copy link
Contributor Author

Could you make PR's title more precise, for example:
[SPARK-35177][SQL] Fix arithmetic overflow in parsing the minimal interval by IntervalUtils.fromYearMonthString

Done

@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42308/

@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42308/

@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42315/

@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42315/

@MaxGekk
Copy link
Member

MaxGekk commented Apr 22, 2021

+1, LGTM. GA passed. Merging to master.
Screenshot 2021-04-22 at 11 56 38
Thank you, @AngersZhuuuu .

@MaxGekk MaxGekk closed this in bb5459f Apr 22, 2021
@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Test build #137765 has finished for PR 32281 at commit 20fdef3.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Test build #137786 has finished for PR 32281 at commit 325549e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 22, 2021

Test build #137779 has finished for PR 32281 at commit 70e9e0a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

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