Skip to content

[SPARK-36049][SQL] Remove IntervalUnit#33265

Closed
AngersZhuuuu wants to merge 4 commits intoapache:masterfrom
AngersZhuuuu:SPARK-36049
Closed

[SPARK-36049][SQL] Remove IntervalUnit#33265
AngersZhuuuu wants to merge 4 commits intoapache:masterfrom
AngersZhuuuu:SPARK-36049

Conversation

@AngersZhuuuu
Copy link
Copy Markdown
Contributor

@AngersZhuuuu AngersZhuuuu commented Jul 8, 2021

What changes were proposed in this pull request?

Remove IntervalUnit

Why are the changes needed?

Clean code

Does this PR introduce any user-facing change?

No

How was this patch tested?

Not need

@github-actions github-actions bot added the SQL label Jul 8, 2021
@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

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

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

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

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

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

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

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

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

Test build #140794 has finished for PR 33265 at commit bf2f4df.

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

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

Test build #140795 has finished for PR 33265 at commit e70a954.

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

@AngersZhuuuu
Copy link
Copy Markdown
Contributor Author

AngersZhuuuu commented Jul 8, 2021

FYI @MaxGekk

MaxGekk
MaxGekk reviewed Jul 8, 2021
View reviewed changes
Copy link
Copy Markdown
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.

LGTM in general except of a comment.

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala Outdated
Comment on lines +765 to +774
def intervalUnits: Seq[UTF8String] = {
Seq(yearStr,
monthStr,
weekStr,
dayStr,
hourStr,
minuteStr,
secondStr,
millisStr,
microsStr)
Copy link
Copy Markdown
Member

@MaxGekk MaxGekk Jul 8, 2021

Choose a reason for hiding this comment

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

Is it used somewhere except of tests? Just wonder why did you move it from ExpressionParserSuite?

Copy link
Copy Markdown
Contributor Author

@AngersZhuuuu AngersZhuuuu Jul 8, 2021

Choose a reason for hiding this comment

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

Is it used somewhere except of tests? Just wonder why did you move it from ExpressionParserSuite?

Move back, how about current?

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

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

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

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

@MaxGekk
Copy link
Copy Markdown
Member

MaxGekk commented Jul 8, 2021

@AngersZhuuuu Could you re-trigger GitHub actions, please.

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

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

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

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

@MaxGekk
Copy link
Copy Markdown
Member

MaxGekk commented Jul 8, 2021

+1, LGTM. Merging to master/3.2.
Thank you, @AngersZhuuuu .

@MaxGekk MaxGekk closed this in fef7e17 Jul 8, 2021
MaxGekk added a commit that referenced this pull request Jul 8, 2021
@AngersZhuuuu @MaxGekk
[SPARK-36049][SQL] Remove IntervalUnit
2f54d9e 
### What changes were proposed in this pull request?
Remove IntervalUnit

### Why are the changes needed?
Clean code

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

### How was this patch tested?
Not need

Closes #33265 from AngersZhuuuu/SPARK-36049.

Lead-authored-by: Angerszhuuuu <angers.zhu@gmail.com>
Co-authored-by: Maxim Gekk <max.gekk@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit fef7e17)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

Test build #140810 has finished for PR 33265 at commit 01d346c.

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

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 8, 2021

Test build #140816 has finished for PR 33265 at commit 5428d0c.

  • 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

Reviewers

@MaxGekk MaxGekk MaxGekk approved these changes

Assignees

No one assigned

Labels

SQL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AngersZhuuuu @SparkQA @MaxGekk