Conversation
sunchao
left a comment
There was a problem hiding this comment.
Correctness
Reviewed 42058679c7ce against 481aefea9c60. The old ignoreList entries suppress both execution and analyzer tests, including Spark-only runs. This change removes those entries and adds a file-level spark.comet.enabled=false setting to the two inputs. SQL statements and golden files stay unchanged. I found no verified P1/P2 issue in the change.
The maintained Spark 3.5/4.0 harnesses parse this header as a configuration setting and apply it to a new session for each test. Comet's scan and execution rules check that session setting before replacing the Spark plan. The setting therefore preserves Spark baseline checks without leaking to other files.
The maintained 4.0 in-set-operations.sql is byte-identical to the input preimage in the 4.1 patch. Its 25 result queries include seven ordered queries, plus three setup statements. Several sort keys leave ties between distinct output rows. For example, the rows with t1c=12, t1a='val1a' differ in t1d, which is absent from the ordering. Spark's harness preserves ordered output and sorts unordered output. This PR leaves that distinction intact and does not rewrite goldens to match Comet. These restored cases exercise Spark. They do not establish native ordering correctness.
Validation
Upstream Spark 4.1 CI passed all four restored cases: execution and analyzer variants of both files. The build and test jobs checked out dfcdc888c720, whose parents are the assigned base and head. Its only change from that base is the reviewed diff file. The affected shard reports 3,879 passed, zero failed, two canceled and 605 ignored. The overall head has 13 successful and 12 skipped checks.
I ran static patch and source-equivalence checks, with no local Spark/JNI build or test. Maintained Spark 3.4/4.1 sources are unavailable, and the alias fixture is absent from the maintained 3.5/4.0 branches. Its complete query semantics could not be independently inspected under the canonical-source constraint. The CI result above is executed Spark 4.1 evidence. The author's local on/off and fork-run claims remain separately unverified.
Performance
This changes test selection and configuration only. It adds no production execution cost or new expression implementation. The additional work is running the two fixtures and their analyzer variants, which the upstream CI log confirms completed. A query microbenchmark would not measure the behavior changed here, and no speedup is claimed.
Design
The existing file-level configuration mechanism directly addresses #5570: restore Spark baseline coverage while retaining an explicit Comet opt-out for ordering-sensitive fixtures. A query-local SQL SET would itself enter the statement/golden sequence, while comment-based settings apply to the whole file. The chosen scope preserves the existing SQL and expected outputs. It also avoids requiring Spark and Comet to choose the same order among tied keys.
Abstraction & complexity
The patch adds no helper, fixture copy or special comparison rule. Both files use the established header convention with a tracking link. Only their two ignore entries are removed. The unrelated ignore entries and existing shared comparison logic remain intact. This is a small, maintainable change with a clear boundary: restored Spark coverage, with native coverage for these fixtures still excluded.

Which issue does this PR close?
Closes #5570.
Rationale for this change
The two ignoreList entries skip Spark baseline tests and analyzer comparisons as well as Comet execution. The ORDER BY tie mismatches still reproduce with Comet.
What changes are included in this PR?
Replace the exclusions in the Spark 4.1.3 diff with file-level Comet opt-outs so both SQL files run against their existing goldens. SQL statements and golden files are unchanged.
How are these changes tested?
Spark 4.1 SQL CI passed on the fork (run). Local targeted execution of the patched SQLQueryTestSuite passed all four cases with Comet enabled and disabled. Verified the regenerated diff applies to the clean Spark tag; Apache RAT passed.