andygrove commented on code in PR #5365: URL: https://github.com/apache/datafusion-comet/pull/5365#discussion_r4242597134
########## docs/source/user-guide/latest/delta.md: ########## @@ -0,0 +1,72 @@ +<!--- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Delta Lake (experimental) + +Comet can execute DSv1 Delta Lake table scans natively. Reads planned by +delta-spark run through Comet's native Parquet scan, inheriting row-group +pruning, page-index pruning, and filter pushdown, with deletion vectors +applied inside the scan. + +Support is experimental and explicitly opt-in. Two things are required: + +1. The `comet-contrib-delta-spark` contrib jar on the classpath, alongside Review Comment: Nit, optional: the release build doesn't pass `-Pdelta` yet (#5882), so there is no published `comet-contrib-delta-spark` jar to put on the classpath. Could this line say the jar has to be built from source for now and point at the README's build steps? The `Delta Lake <delta>` entry in `index.rst` also sets its own title, so the sidebar won't show "(experimental)" the way the page heading does. ########## dev/ci/compute-changes.py: ########## @@ -577,6 +604,12 @@ "iceberg_1_9": ["nightly", "label:run-iceberg-tests"], "iceberg_1_10": ["nightly", "label:run-iceberg-tests"], "iceberg_1_11": ["queue", "label:run-iceberg-tests"], + # Same shape for the Delta contrib suites: Spark 3.5 is the profile the + # MinIO and feature-off jobs already pin, so it is the one the queue runs; + # 4.0 and 4.1 run nightly. One label opts a pull request into all three. + "delta_3_5": ["queue", "label:run-delta-tests"], Review Comment: This makes `delta_3_5` a required queue job for most PRs. Replaying this filter over the last 40 merges on `main`, it would have run for 30 of them, the same set as `spark_4_1`. Nothing in the PR tier builds with `-Pdelta`, though. A core change that breaks the contrib passes its own PR run and first fails in the queue, where it gets evicted and the PRs behind it are rebuilt. The contrib calls a lot of core internals (`QueryPlanSerde`, `CometNativeScan`, `CometScanRule`, `CometExecRDD`, `NativeConfig` and more), so I'd expect this to come up. The Rust test you added for the S3 keys covers one case of it, not the general one. I suggested keeping one Delta profile in the queue back in September, but I'd like to revisit that while the contrib is experimental. Could `delta_3_5` move to the nightly tier with 4.0 and 4.1, still behind `run-delta-tests`? And could the Spark 3.5 strict warnings compile in `pr_build_linux.yml` add the `delta` profile, so a change that breaks the contrib build fails on its own PR? I tried `./mvnw test-compile -Pspark-3.5,delta -Pstrict-warnings -DskipTests` on the merge with `main`, and the contrib compiles cleanly, including the new unused-params lint from #6846. -- 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]
