[SPARK-29873][SQL][Test][FOLLOWUP] set operations should not escape when regen golden file with --SET --import both specified

[yaooqinn]

What changes were proposed in this pull request?

When regenerating golden files, the set operations via --SET will not be done, but those with --import should be exceptions because we need the set command.

Why are the changes needed?

fix test tool.

Does this PR introduce any user-facing change?

How was this patch tested?

add ut, but I'm not sure we need these tests for tests itself.
cc @maropu @cloud-fan

[SparkQA]

Test build #113928 has finished for PR 26557 at commit a854316.

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

[SparkQA]

Test build #113930 has finished for PR 26557 at commit c791b36.

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

[SparkQA]

Test build #113937 has finished for PR 26557 at commit 8fb683e.

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

[cloud-fan]

ah seems we have 2 different kinds of settings now:

1. settings that shouldn't change result. We want to generate the golden files without the settings and run tests with settings.
2. setting that can change result.

The second one is pretty useful with --import. We want to run the same queries with different settings and save the answers.

Shall we have different directives for them? @maropu

[cloud-fan]

After a second thought, we only expect the --SET to change behavior if we import queries. So the change here LGTM.

@yaooqinn can you update the comment to explain it?

[cloud-fan]

BTW now --SET is upper case and --import is lower case. Seems better to always use upper case.

[cloud-fan]

also cc @wangyum , why do we need isTestWithConfigSets for thriftserver tests?

[yaooqinn]

OK, I will update the comments and change case for --import

[wangyum]

@cloud-fan To reduce the test time. The mvn test often timed out at that time: https://amplab.cs.berkeley.edu/jenkins/job/spark-master-test-maven-hadoop-2.7/

[cloud-fan]

do you mean setting config is expensive?

[wangyum]

It will run configSets times:

spark/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala

Lines 287 to 298 in ec5d698

	configSets.foreach { configSet =>
	try {
	runQueries(queries, testCase, Some(configSet))
	} catch {
	case e: Throwable =>
	val configs = configSet.map {
	case (k, v) => s"$k=$v"
	}
	logError(s"Error using configs: ${configs.mkString(",")}")
	throw e
	}
	}

[cloud-fan]

ah that's a problem. We should think about how to reduce test time.

[maropu]

Oh, I'm late. The change looks reasonable...

[SparkQA]

Test build #114006 has finished for PR 26557 at commit 538a44f.

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

[SparkQA]

Test build #114007 has finished for PR 26557 at commit 3192e15.

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

[cloud-fan]

thanks, merging to master!

[maropu]

late LGTM