Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ ruleset in `.asf.yaml`. That splits CI into three tiers:
- **Nightly tier** (`nightly`): the regression sweep of everything else, once
a day against `main` as it stands. The Comet test suites against the other
three Spark profiles, Spark SQL on Spark 3.5, 4.0 and 4.2, Iceberg
1.9/1.10, and the Delta contrib suites on Spark 3.5, 4.0 and 4.1. See
1.9/1.10, and the Delta contrib suites on Spark 3.5, 4.0, 4.1 and 4.2. See
[Nightly tier](#nightly-tier) below for how a failure surfaces.

Every queue-only and nightly job has a `run-*` label that opts a pull request
Expand Down Expand Up @@ -122,6 +122,7 @@ tiers partition the list and that the `pr` tier is exactly the default profile.
delta_3_5 run-delta-tests
delta_4_0 run-delta-tests
delta_4_1 run-delta-tests
delta_4_2 run-delta-tests

| | |
+-----------------------------------+-----------------------------------+
Expand Down Expand Up @@ -161,6 +162,7 @@ tiers partition the list and that the `pr` tier is exactly the default profile.
| `delta_3_5` | nightly, **or** PR with `run-delta-tests` | Delta contrib and native sources |
| `delta_4_0` | nightly, **or** PR with `run-delta-tests` | Delta contrib and native sources |
| `delta_4_1` | nightly, **or** PR with `run-delta-tests` | Delta contrib and native sources |
| `delta_4_2` | nightly, **or** PR with `run-delta-tests` | Delta contrib and native sources |
| `docs` | push to main, paths matched, **or** dispatch on `main` | `.asf.yaml`, `docs/**`, `docs.yaml` |
| `spark_3_5` | nightly, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources |
| `spark_4_1` | PR or merge group for catalyst and `sql_core`; the `sql_hive` shards in the merge group, **or** PR with `run-spark-4.1-hive-tests` | Spark 4.1 sources |
Expand Down Expand Up @@ -379,16 +381,16 @@ umbrella doesn't watch, or operate independently of the rest of CI:

## Reusable workflows (called by `ci.yml`)

| File | Called from `ci.yml` job(s) |
| ----------------------------- | ------------------------------------- |
| `pr_build_linux.yml` | `pr_build_linux` |
| `pr_build_macos.yml` | `pr_build_macos` |
| `pr_benchmark_check.yml` | `pr_benchmark_check` |
| `delta_build_gate.yml` | `delta_build_gate` |
| `delta_contrib_test.yml` | `delta_3_5`, `delta_4_0`, `delta_4_1` |
| `pyarrow_udf_test.yml` | `pyarrow_udf_test` |
| `docs.yaml` | `docs` |
| `spark_sql_test_reusable.yml` | `iceberg_spark_test_reusable.yml` |
| File | Called from `ci.yml` job(s) |
| ----------------------------- | -------------------------------------------------- |
| `pr_build_linux.yml` | `pr_build_linux` |
| `pr_build_macos.yml` | `pr_build_macos` |
| `pr_benchmark_check.yml` | `pr_benchmark_check` |
| `delta_build_gate.yml` | `delta_build_gate` |
| `delta_contrib_test.yml` | `delta_3_5`, `delta_4_0`, `delta_4_1`, `delta_4_2` |
| `pyarrow_udf_test.yml` | `pyarrow_udf_test` |
| `docs.yaml` | `docs` |
| `spark_sql_test_reusable.yml` | `iceberg_spark_test_reusable.yml` |

## Changing what runs when

Expand Down
17 changes: 15 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ jobs:
delta_3_5: ${{ steps.compute.outputs.delta_3_5 }}
delta_4_0: ${{ steps.compute.outputs.delta_4_0 }}
delta_4_1: ${{ steps.compute.outputs.delta_4_1 }}
delta_4_2: ${{ steps.compute.outputs.delta_4_2 }}
steps:
- uses: actions/checkout@v7
with:
Expand Down Expand Up @@ -524,14 +525,25 @@ jobs:
needs: changes
permissions:
contents: read
# Nightly by default; PRs need the `run-delta-tests` label. No Spark 4.2
# caller: the contrib profile is dormant until a Delta release supports it.
# Nightly by default; PRs need the `run-delta-tests` label.
if: needs.changes.outputs.delta_4_1 == 'true'
uses: ./.github/workflows/delta_contrib_test.yml
with:
spark: '4.1'
java: '17'

delta_4_2:
name: Delta Contrib Tests (Spark 4.2)
needs: changes
permissions:
contents: read
# Nightly by default; PRs need the `run-delta-tests` label.
if: needs.changes.outputs.delta_4_2 == 'true'
uses: ./.github/workflows/delta_contrib_test.yml
with:
spark: '4.2'
java: '17'

spark_4_0:
name: Spark SQL Tests (Spark 4.0)
needs: changes
Expand Down Expand Up @@ -673,6 +685,7 @@ jobs:
- delta_3_5
- delta_4_0
- delta_4_1
- delta_4_2
- spark_4_0
- spark_4_1
- spark_4_2
Expand Down
14 changes: 7 additions & 7 deletions .github/workflows/delta_contrib_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,9 @@ jobs:
# The container job above has no Docker socket, so CometDeltaS3Suite cancels itself
# there. This job runs it on the plain runner, where Testcontainers can start MinIO.
contrib-delta-s3:
name: Delta contrib S3 suite (Spark 3.5, MinIO)
# Pinned to Spark 3.5, so only that caller runs it: once per ci.yml run.
if: inputs.spark == '3.5'
name: Delta contrib S3 suite (Spark ${{ inputs.spark }}, MinIO)
# Cover the oldest pairing and Hadoop 3.5's encryption-context initialization.
if: inputs.spark == '3.5' || inputs.spark == '4.2'
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v7
Expand Down Expand Up @@ -167,7 +167,7 @@ jobs:
with:
path: |
~/.m2/repository
key: ${{ runner.os }}-delta-maven-${{ hashFiles('**/pom.xml') }}-3.5
key: ${{ runner.os }}-delta-maven-${{ hashFiles('**/pom.xml') }}-${{ inputs.spark }}
# Its own prefix: the Spark SQL and Comet test jobs fall back to the
# newest `java-maven-` entry, and this repository (built with
# `-Pdelta`) broke Spark's Arrow tests when they restored it.
Expand Down Expand Up @@ -200,12 +200,12 @@ jobs:

- name: Install Comet core jars
run: |
./mvnw -B -q -Prelease -Pspark-3.5,delta install -pl common,spark -DskipTests -Dspotless.check.skip=true
./mvnw -B -q -Prelease -Pspark-${{ inputs.spark }},delta install -pl common,spark -DskipTests -Dspotless.check.skip=true

- name: Run the MinIO-backed S3 suite
run: |
docker info > /dev/null
SPARK_HOME=$(pwd) COMET_CONF_DIR=$(pwd)/conf ./mvnw -B -Prelease -Pspark-3.5,delta test -pl contrib/delta-spark -DwildcardSuites=org.apache.comet.contrib.delta.CometDeltaS3Suite
SPARK_HOME=$(pwd) COMET_CONF_DIR=$(pwd)/conf ./mvnw -B -Prelease -Pspark-${{ inputs.spark }},delta test -pl contrib/delta-spark -DwildcardSuites=org.apache.comet.contrib.delta.CometDeltaS3Suite
env:
# This job exists only to run the suite, so a Docker or MinIO failure must fail
# it instead of cancelling every test, which the container job above relies on.
Expand All @@ -218,7 +218,7 @@ jobs:
with:
path: |
~/.m2/repository
key: ${{ runner.os }}-delta-maven-${{ hashFiles('**/pom.xml') }}-3.5
key: ${{ runner.os }}-delta-maven-${{ hashFiles('**/pom.xml') }}-${{ inputs.spark }}

# `delta` is in the default feature set, so no regular job builds without it;
# this keeps the feature-off build and its "built without the delta feature"
Expand Down
14 changes: 12 additions & 2 deletions contrib/delta-spark/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ for configuration details.
| 4.0 | 4.0.x | supported |
| 4.1 | 4.3.x | supported |
| 3.4 | delta-core 2.4 | not supported (older Delta, would need shims) |
| 4.2 | none released | inert until Delta ships a Spark 4.2 release |
| 4.2 | 4.4.x | supported |

Spark 4.2 uses the `delta-spark_4.2_2.13` artifact, pinned to Delta 4.4.1 by
the `spark-4.2` Maven profile. Earlier Spark profiles keep `delta-spark` with
their existing version pins.

## Building and testing

Expand All @@ -60,11 +64,17 @@ install is the trap the contributor guide warns about:

Run the test suites the same way (`test` instead of `install` on the second
line). CI runs them via `.github/workflows/delta_contrib_test.yml`: nightly on
Spark 3.5, 4.0 and 4.1, or on a pull request carrying the `run-delta-tests`
Spark 3.5, 4.0, 4.1 and 4.2, or on a pull request carrying the `run-delta-tests`
label.
`CometDeltaS3Suite` starts MinIO through Testcontainers and cancels itself when
no Docker daemon is reachable; setting `COMET_DELTA_S3_REQUIRED=1`, as the
`contrib-delta-s3` CI job does, turns that cancel into a suite failure.

`dev/` contains a benchmark script (`bench_delta_comet.py`) and a harness for
running Delta's own test suites against Comet (`run-delta-regression.sh`).

### S3 encryption context

A nonempty effective `fs.s3a.encryption.context` makes the Delta scan fall back to Spark. The fallback also applies on older Hadoop versions that ignore the key. The context is attached to KMS write requests, but Hadoop 3.5 also parses it while initializing an S3 filesystem for reads. A local Delta log or shallow clone can reference S3 data files without initializing that data bucket during metadata reads. Comet therefore declines configured contexts, including valid ones, so Spark performs the same initialization checks before opening those files. Bucket aliases and credential-provider values are included; decline reasons do not print context values. Existing SSE-C and client-side encryption declines still apply.

The MinIO CI jobs run on Spark 3.5 and 4.2. The Spark 4.2 case covers a local shallow clone whose files remain in S3, with valid and malformed global, short-bucket and long-bucket context settings. The unconfigured control must use the native Delta scan; configured reads must fall back, returning rows for valid context and failing like Spark for malformed context.
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,12 @@ object DeltaScanSupport {
return encryptionReason
}

val contextReason =
unsupportedEncryptionContextReason(hadoopConf, dataFileUris ++ dvUris, propagatedConfCache)
if (contextReason.isDefined) {
return contextReason
}

// Always zero-I/O (plain propagated-conf read, no keystore): native's S3 client has no
// HTTP proxy support at all (no fs.s3a.proxy.* key is read anywhere in s3.rs), so a bucket
// requiring a proxy for S3 egress must decline here rather than claim and then connect
Expand Down Expand Up @@ -1155,6 +1161,42 @@ object DeltaScanSupport {
}
}

/**
* Decline S3 encryption context because native does not reproduce Hadoop's setup-time parsing.
* A local Delta log can reference S3 data or DVs, so metadata reads need not initialize S3A.
* Resolve the same bucket aliases and credential providers as Hadoop, without exposing values.
*/
private[delta] def unsupportedEncryptionContextReason(
hadoopConf: Configuration,
uris: Seq[URI],
propagatedConfCache: MutableMap[String, Configuration] = MutableMap.empty)
: Option[String] = {
val key = "fs.s3a.encryption.context"
uris
.flatMap(s3Bucket)
.distinct
.iterator
.map { bucket =>
try {
val propagatedConf = propagatedConfCache.getOrElseUpdate(
bucket,
propagateBucketOptions(hadoopConf, bucket))
hadoopLookupPasswordEffective(propagatedConf, bucket, key) match {
case Left(reason) => Some(reason)
case Right(Some(_)) =>
Some(
s"Native Delta scan does not support $key for $bucket " +
"(Hadoop validates the context during S3A initialization)")
case Right(None) => None
}
} catch {
case e @ (_: IOException | _: RuntimeException) =>
Some(unverifiableValueReason(bucket, key, e))
}
}
.collectFirst { case Some(reason) => reason }
}

private def encryptionAlgorithmReason(conf: Configuration, bucket: String): Option[String] =
effectiveEncryptionAlgorithm(conf, bucket) match {
case Left(reason) => Some(reason)
Expand Down Expand Up @@ -1522,22 +1564,22 @@ object DeltaScanSupport {
* for the Arm A/B/C dispatch this delegates to) -- or `Left(reason)` when the value cannot be
* safely verified.
*
* USED ONLY for keys whose real consumer IS `lookupPassword` -- the [[LookupPasswordConsumer]]
* entries of [[S3ConfigKeyConsumers]], via [[s3KeyDivergenceReason]]. An earlier version ran
* EVERY compared key through this function, reasoning that a wider read could only ever
* over-decline; for a value-EQUALITY comparator that reasoning is half-true: the long-form
* alias this function consults FIRST can hold exactly the value native resolves while Hadoop's
* true propagate-then-plain-get value differs (e.g. a `${...}` reference whose referent
* propagation redirects), producing a false EQUALITY that admits a really-diverging scan. A
* [[PropagatedOptionConsumer]] key must therefore resolve like its actual consumer instead --
* see [[s3KeyDivergenceReason]].
* Used for keys whose real consumer is `lookupPassword`: the [[LookupPasswordConsumer]] entries
* of [[S3ConfigKeyConsumers]] and the dedicated [[unsupportedEncryptionContextReason]] gate. An
* earlier version ran EVERY compared key through this function, reasoning that a wider read
* could only ever over-decline; for a value-EQUALITY comparator that reasoning is half-true:
* the long-form alias this function consults FIRST can hold exactly the value native resolves
* while Hadoop's true propagate-then-plain-get value differs (e.g. a `${...}` reference whose
* referent propagation redirects), producing a false EQUALITY that admits a really-diverging
* scan. A [[PropagatedOptionConsumer]] key must therefore resolve like its actual consumer
* instead -- see [[s3KeyDivergenceReason]].
*
* Honors [[ClearTextFallbackKey]] (via [[resolveViaCredentialAliases]]'s [[NoProvider]] arm),
* matching `getPassword`'s real refusal to read plaintext conf when the flag is off.
*
* `hadoopConf` must be a [[propagateBucketOptions]] result (the caller,
* [[s3KeyDivergenceReason]], always passes one) so that `${...}` references embedded in any
* alias resolve exactly like `S3AFileSystem#initialize`'s real propagate-then-resolve order.
* `hadoopConf` must be a [[propagateBucketOptions]] result (both callers pass one) so that
* `${...}` references embedded in any alias resolve exactly like `S3AFileSystem#initialize`'s
* real propagate-then-resolve order.
*/
private def hadoopLookupPasswordEffective(
hadoopConf: Configuration,
Expand Down
Loading