Skip to content

Conversation

@yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented May 27, 2020

What changes were proposed in this pull request?

This PR modifies some codegen related tests to test escape characters for datetime functions which are time zone aware. If the timezone is absent, the formatter could result in null caused by java.util.NoSuchElementException: None.get and bypassing the real intention of those test cases.

Why are the changes needed?

fix tests

Does this PR introduce any user-facing change?

no

How was this patch tested?

passing the modified test cases.

@yaooqinn yaooqinn changed the title [SPARK-31835][SQL][TESTS] Add zoneId to codegen related test in DateExpressionsSuite [SPARK-31835][SQL][TESTS] Add zoneId to codegen related tests in DateExpressionsSuite May 27, 2020
@yaooqinn
Copy link
Member Author

cc @cloud-fan @MaxGekk thanks very much.

}
// Test escaping of format
GenerateUnsafeProjection.generate(FromUnixTime(Literal(0L), Literal("\"quote")) :: Nil)
GenerateUnsafeProjection.generate(FromUnixTime(Literal(0L), Literal("\"quote"), JST_OPT) :: Nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

if the timezone doesn't matter, shall we use UTC?

Copy link
Member

Choose a reason for hiding this comment

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

Particular time zone doesn't matter here, we just check that codegen doesn't throw any exception.

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.

LGTM

}
// Test escaping of format
GenerateUnsafeProjection.generate(FromUnixTime(Literal(0L), Literal("\"quote")) :: Nil)
GenerateUnsafeProjection.generate(FromUnixTime(Literal(0L), Literal("\"quote"), JST_OPT) :: Nil)
Copy link
Member

Choose a reason for hiding this comment

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

Particular time zone doesn't matter here, we just check that codegen doesn't throw any exception.

@SparkQA
Copy link

SparkQA commented May 27, 2020

Test build #123178 has finished for PR 28653 at commit 5eea688.

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

@SparkQA
Copy link

SparkQA commented May 27, 2020

Test build #123186 has finished for PR 28653 at commit db92498.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master/3.0!

@cloud-fan cloud-fan closed this in 311fe6a May 27, 2020
cloud-fan pushed a commit that referenced this pull request May 27, 2020
…ExpressionsSuite

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

This PR modifies some codegen related tests to test escape characters for datetime functions which are time zone aware. If the timezone is absent, the formatter could result in `null` caused by `java.util.NoSuchElementException: None.get` and bypassing the real intention of those test cases.

### Why are the changes needed?

fix tests

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

no

### How was this patch tested?

passing the modified test cases.

Closes #28653 from yaooqinn/SPARK-31835.

Authored-by: Kent Yao <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 311fe6a)
Signed-off-by: Wenchen Fan <[email protected]>
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