Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Nov 2, 2019

What changes were proposed in this pull request?

In the PR, I propose to changed CalendarInterval.toString:

  • to skip the week unit
  • to convert milliseconds and microseconds as the fractional part of the seconds unit.

Why are the changes needed?

To improve readability.

Does this PR introduce any user-facing change?

Yes

How was this patch tested?

  • By CalendarIntervalSuite and IntervalUtilsSuite
  • literals.sql, datetime.sql and interval.sql

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 2, 2019

ping @cloud-fan @dongjoon-hyun

@SparkQA
Copy link

SparkQA commented Nov 2, 2019

Test build #113127 has finished for PR 26367 at commit 12d3583.

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

@SparkQA
Copy link

SparkQA commented Nov 2, 2019

Test build #113130 has finished for PR 26367 at commit 662eeef.

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

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 3, 2019

jenkins, retest this, please

@SparkQA
Copy link

SparkQA commented Nov 3, 2019

Test build #113157 has finished for PR 26367 at commit 662eeef.

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

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 3, 2019

jenkins, retest this, please

@SparkQA
Copy link

SparkQA commented Nov 3, 2019

Test build #113167 has finished for PR 26367 at commit 662eeef.

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

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, @MaxGekk , @cloud-fan , @maropu .
This looks really nicer.

Merged to master.

@MaxGekk MaxGekk deleted the interval-to-string-format branch June 5, 2020 19:40
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.

5 participants