Repository navigation
test: let Comet SQL tests keep ConstantFolding, exclude optimizer rules and change configs mid-file #6618
Description
Part of #6615.
What is the problem the feature request solves?
CometSqlFileTestSuite runs each fixture under withSQLConf(ansiDisabled ++ file.configs ++ constantFoldingExcluded). Because the suite's spark.sql.optimizer.excludedRules=...ConstantFolding comes last, a fixture cannot change the optimizer rules:
- A fixture cannot exclude another rule.
-- Config: spark.sql.optimizer.excludedRules=...is silently overwritten. Tests that need a rule out of the way have to stay in Scala:NullPropagation, so a NULL literal reaches the kernel instead of being folded to a typed NULL:hll_unionwith a NULL flag, rlike with a NULL pattern.ConvertToLocalRelation, so an expression over an inlineVALUESrelation runs in Comet with non-nullable elements (bug: array/map kernels crash on non-null child field via CometLocalTableScanExec #4789):slice,array_insert,map_entries.EliminateSorts, so aSortunder an order-sensitive aggregate survives: two decimal SUM/AVG tests.
- A fixture cannot keep ConstantFolding. The whole family of folded-literal tests needs Spark to fold
array(...),map(...)ornamed_struct(...)into anArrayType/MapType/StructTypeLiteral, so thatCometLiteral's expansion and decline logic runs. These are 31 tests inCometArrayExpressionSuiteandCometMapExpressionSuite, plus a few inCometExpressionSuiteandCometRegExpJvmSuite(nested-literalINcandidates, collated literal patterns). With ConstantFolding excluded, the same SQL reachesCreateArray/CreateMap/CreateNamedStructinstead, which is a different code path.
The audit for #6615 counted about 45 tests blocked this way.
There is a workaround today, but it is fragile. A mid-file statement SET spark.sql.optimizer.excludedRules=... takes effect and is restored afterwards, because the key is in the suite's own config list (cast/cast_timestamp_signed_year.sql does this for ANSI). A SET of any key that is not also declared in Config / ConfigMatrix is never restored. It leaks into every fixture that runs later in the same session, so it cannot be recommended.
Describe the potential solution
- A directive that adds rules to the exclusion list. Merge it with ConstantFolding rather than replacing it, e.g.
-- ExcludeRules: org.apache.spark.sql.catalyst.optimizer.NullPropagation. - A directive that keeps ConstantFolding for a file, e.g.
-- ConstantFolding: enabled. Document that all-literal queries in such a file are folded before Comet sees them, except the folded-literal shapes they exist to test. - Make mid-file
statement SETsafe. Either record every key aSETchanges and restore it when the file ends, or reject aSETof a key the file does not declare. This also gives a supported way to switch ANSI or a Comet config between queries in one file, which several conversions need. - Optional: clean up
CREATE [OR REPLACE] [TEMP] VIEWnames the wayCREATE TABLEnames are cleaned up. Several Scala tests use temp views; for now conversions use subqueries. - Fix the
Config/ConfigMatrixdirective parser. It uses--\s*Config:\s*(.+)=(.+), and the greedy first group splits on the last=that leaves something after it. Any value containing=is mis-parsed. For example,-- Config: spark.sql.parquet...key=MDEyMzQ1Njc4OTAxMjM0NQ==sets a key named...key=MDEyMzQ1Njc4OTAxMjM0NQto=. Split on the first=instead. The Variant encryption test needs this.
Document all of these in docs/source/contributor-guide/sql-file-tests.md.
Activity