andygrove opened a new pull request, #6963:
URL: https://github.com/apache/datafusion-comet/pull/6963
## Which issue does this PR close?
None. This turns the default from #5634 back off for the 1.2.0 release,
because of #6787, #6898 and #6922. Part of #6834.
## Rationale for this change
Since #5634 made `spark.comet.exec.inMemoryCache.enabled` default to `true`
on Spark 3.5 and later, three open wrong-results bugs are reachable with
default settings. On branch-1.1, where the cache is opt-in, none of them is:
| Issue | With default settings | Fix |
| --- | --- | --- |
| #6787 | An outer hash join over a sorted cache returns unsorted partitions
| #6835, in review |
| #6898 | A broadcast nested loop join over a sorted cache does the same.
With AQE off, a sort-merge join above it returns 10600 rows where Spark returns
30000 | None yet |
| #6922 | Cache shapes that #6835 still leaves out of order | None yet |
They share a cause. Comet operators report the order Spark reports for them
without always keeping it natively, and a cached relation carries that order
into later queries, where nothing checks it. Each review round on #6835 has
found another case, most recently a cache written through a native sample. With
default settings, the cache is the main way a sorted input reaches a native
plan, so turning it off removes that exposure while the fixes land.
The default has been on in `main` for four days, and 1.2.0 would be the
first release to ship it. 1.1.0 shipped the cache as opt-in, so with this
change no release has it on by default, and the 1.2.0 upgrade guide no longer
needs a section for it. Turning it back on can wait for #6787, #6898 and #6922
to be fixed, ideally early in a release cycle so it gets a full cycle on `main`
before it ships.
## What changes are included in this PR?
- `spark.comet.exec.inMemoryCache.enabled` defaults to `false` on every
Spark version. The Spark 3.4 special case goes away. The 3.4 union limitation
it described stays in the config doc and the user guide for anyone who turns
the cache on.
- Tests keep running against Comet's cache format:
- `CometInMemoryCacheSuite` sets the flag in its `sparkConf`. The other
cache suites already set it themselves. The test that pinned the per-version
default now asserts that it is `false`.
- The 3.5.9, 4.0.4, 4.1.3 and 4.2.0 Spark SQL diffs set the flag beside
Comet's cache serializer in `CometTestSettings`, `SharedSparkSession` and
`TestHive`, so Spark's suites keep running against Comet's cache format.
Without the flag they would write Comet's format and read it back through
Spark's fallback scan. The 3.4.3 diff is unchanged.
- Docs: the user guide, the operator tables and the plan guide describe the
cache as experimental and disabled by default again. The In-Memory Cache
guide's Limitations section describes the ordering issues and links them. The
1.2.0 upgrade guide section is removed, and the Spark SQL tests guide explains
why the test JVMs turn the cache on.
## How are these changes tested?
- `CometInMemoryCacheSuite`, `CometInMemoryCachePruningSuite`,
`CometInMemoryCacheKryoSuite`, `CometInMemoryCacheKryoUnregisteredSuite` and
`CometInMemoryCacheKryoClassesToRegisterSuite` pass locally on Spark 4.1 (80
tests) and Spark 3.4 (74 tests, and 6 canceled that need Spark 3.5 or 4.0).
- Each diff was regenerated from a clean checkout of its tag with the
current diff applied. Before the edit, the regenerated diff matched the current
one byte for byte. After it, the diff changes only in the lines above, and it
applies cleanly to a fresh checkout of the tag.
- `run-all-spark-profiles` and the `run-spark-3.5-tests`,
`run-spark-4.0-tests`, `run-spark-4.1-tests` and `run-spark-4.2-tests` labels
run Comet's suites and Spark's SQL suites before this is queued.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
