Skip to content

Comments

[SPARK-38316][SQL][TESTS] Fix SQLViewSuite/TriggerAvailableNowSuite/Unwrap*Suite under ANSI mode#35652

Closed
gengliangwang wants to merge 3 commits intoapache:masterfrom
gengliangwang:fix
Closed

[SPARK-38316][SQL][TESTS] Fix SQLViewSuite/TriggerAvailableNowSuite/Unwrap*Suite under ANSI mode#35652
gengliangwang wants to merge 3 commits intoapache:masterfrom
gengliangwang:fix

Conversation

@gengliangwang
Copy link
Member

What changes were proposed in this pull request?

Fix the following test suites under ANSI mode:

  • TriggerAvailableNowSuite
  • GlobalTempViewTestSuite
  • LocalTempViewTestSuite
  • PersistedViewTestSuite
  • SimpleSQLViewSuite
  • UnwrapCastInComparisonEndToEndSuite
  • UnwrapCastInBinaryComparisonSuite

Why are the changes needed?

To set up a new GA test job with ANSI mode on

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manually turn on ANSI mode and test .
Also it should pass GA tests.

def verifyCastBoolean(df: DataFrame, query: String, answer: Seq[Row]): Unit =
if (conf.ansiEnabled) {
intercept[AnalysisException] {
df.where(query).collect()
Copy link
Contributor

@cloud-fan cloud-fan Feb 25, 2022

Choose a reason for hiding this comment

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

It looks meaningless to test ansi error behavior in this optimizer rule test suite. Can we simply turn off ansi in this test case?

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

val q = startQuery()

def makeDataFrame(a: Seq[Int]): DataFrame = if (SQLConf.get.ansiEnabled) {
a.map(_.toLong).toDF()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we always use .toLong in this test?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for making the expected answer of Long union String

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, this streaming test wants to check Long union String?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, let's update the test case instead

.text(src.getCanonicalPath)

val df2 = testSource.toDF
val df2 = testSource.toDF.selectExpr("cast(value as string)")
Copy link
Member Author

Choose a reason for hiding this comment

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

explicit cast as string so that we don't need to concern the type coercion result of long union string

@gengliangwang
Copy link
Member Author

@cloud-fan @HyukjinKwon @HeartSaVioR Thanks for the reviews!
Merging to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants