parthchandra commented on PR #4366:
URL:
https://github.com/apache/datafusion-comet/pull/4366#issuecomment-5320446796
Some follow up items from A.2 (#4952) -
**1. CI doesn't actually compile the Delta Scala until A.6a.** CI never
runs a `-Pcontrib-delta` Scala compile. Right now that's checked by hand);
between A.2 and A.6a, if someone changes `CometScanContrib` or
`CometContribScanMarker` in core, they could break the Delta Scala and CI
wouldn't catch it.
**2. `DeltaConf` shows up before the things it controls exist.** A.2 adds
`DeltaConf` (the table lists it under A.4a), and its settings only start doing
anything in A.3–A.5:
- `spark.comet.scan.deltaNative.enabled` defaults to `true`. It doesn't
do anything today, but once A.3b/A.4b are done it'll turn a native path on by
default while it's still experimental. I'd default it to `false` until the
feature is solid.
- `spark.comet.delta.cdf.maxPartitions` is already here even though CDF
isn't until A.5, and it's under a different prefix (`spark.comet.delta.cdf.*`)
than the other three (`spark.comet.scan.deltaNative.*`). Let's settle on one
prefix, and maybe hold the CDF setting back to A.5.
- `dataFileConcurrencyLimit` says "2 to 8 is typical" in its docs but
defaults to `1` and only checks that it's positive.
**3. The delta-spark version pin isn't tracked anywhere.** A.2's
description mentions the delta-spark 4.1.1 pin is a decision we're putting off,
but there's no issue for it, and the gate script already hard-codes spark-4.1 →
delta-spark 4.1.x.
**4. These configs won't make it into the generated docs.** `DeltaConf`
lives outside `CometConf`, so `GenerateDocs` won't see them.
**5. (Optional) noticing when two contribs grab the same scan.** The
"first claim wins" rule is just documented, not enforced. If we want core to
actually warn about a misconfigured setup, the debug-only "ask everyone and log
if more than one claims" idea from the A.2 thread would do it.
--
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]