Skip to content

Commit eb544d6

Browse files
andygroveandygrove
committed
Merge remote-tracking branch 'apache/main' into prototype-jvm-scalar-udf
2 parents ce01339 + 5910560 commit eb544d6

942 files changed

Lines changed: 18394 additions & 54272 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/skills/bug-triage/SKILL.md‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
---
2+
name: bug-triage
3+
description: Triage open Comet issues marked `requires-triage` per the project bug triage guide. Applies the recommended priority and area labels, removes `requires-triage`, and files a dated summary issue listing what was done. A human reviews the summary issue and closes it when satisfied.
4+
---
5+
6+
Run a bug triage pass for the `apache/datafusion-comet` repository.
7+
8+
## Overview
9+
10+
This skill triages every open issue carrying the `requires-triage` label. For
11+
each one it:
12+
13+
1. Decides a priority and area labels using the project's triage guide.
14+
2. Applies those labels via `gh`.
15+
3. Removes the `requires-triage` label.
16+
4. Records the decision (with rationale) in a single dated summary issue.
17+
18+
A human reviewer reads the summary issue, sanity-checks the calls, and closes
19+
it when satisfied. Any label correction is done by the reviewer directly on the
20+
affected issue.
21+
22+
The triage criteria come from the project's own guide. Read it before doing any
23+
classification work; do not rely on memory.
24+
25+
## Step 1: Read the Triage Guide
26+
27+
Read the canonical guide in this repository:
28+
29+
```
30+
docs/source/contributor-guide/bug_triage.md
31+
```
32+
33+
Use the priority decision tree, escalation triggers, area labels, and
34+
prioritization principles from that guide. If the guide and this skill ever
35+
disagree, the guide wins. Do not paraphrase the guide; quote the labels and
36+
criteria verbatim when classifying.
37+
38+
## Step 2: Gather Issues That Need Triage
39+
40+
Fetch all open issues labeled `requires-triage`:
41+
42+
```bash
43+
gh issue list \
44+
--repo apache/datafusion-comet \
45+
--label requires-triage \
46+
--state open \
47+
--limit 200 \
48+
--json number,title,author,createdAt,labels,body,url
49+
```
50+
51+
If the list is empty, stop and tell the user there is nothing to triage. Do not
52+
file an empty summary issue and do not modify any labels.
53+
54+
## Step 3: Classify Each Issue
55+
56+
For each issue, review the title and body and determine:
57+
58+
1. **Priority label** (exactly one): apply the decision tree from the guide.
59+
- `priority:critical` for correctness issues (silent wrong results, data
60+
corruption) and security vulnerabilities
61+
- `priority:high` for crashes, panics, segfaults, NPEs on supported paths
62+
- `priority:medium` for functional bugs / performance regressions with
63+
workarounds
64+
- `priority:low` for test-only, CI flakes, tooling, cosmetic
65+
2. **Area labels** (zero or more): from the area table in the guide
66+
(`area:writer`, `area:shuffle`, `area:aggregation`, `area:scan`,
67+
`area:expressions`, `area:ffi`, `area:ci`) plus the pre-existing area
68+
indicators (`native_datafusion`, `native_iceberg_compat`, `spark 4`,
69+
`spark sql tests`).
70+
3. **Escalation note**: if the issue matches an escalation trigger from the
71+
guide (e.g., a `priority:high` crash that may also produce wrong results),
72+
note it in the summary.
73+
74+
## Step 4: Skip Issues You Cannot Confidently Classify
75+
76+
If an issue lacks reproduction steps or is otherwise too ambiguous to classify
77+
with confidence:
78+
79+
- **Do not** apply a priority label.
80+
- **Do not** remove `requires-triage`.
81+
- **Do not** comment on the issue or ask the reporter for more info from this
82+
skill (that is the human reviewer's call).
83+
- Record it in the summary under a "Skipped — needs more info" section so the
84+
reviewer can follow up.
85+
86+
Guessing is worse than skipping.
87+
88+
## Step 5: Apply Labels
89+
90+
For each issue you classified in Step 3, apply the labels and remove
91+
`requires-triage` in a single `gh` call:
92+
93+
```bash
94+
gh issue edit <NUMBER> \
95+
--repo apache/datafusion-comet \
96+
--add-label "priority:high,area:expressions" \
97+
--remove-label "requires-triage"
98+
```
99+
100+
Notes:
101+
102+
- Pass the labels as a single comma-separated string (no spaces around commas).
103+
- Quote labels that contain spaces (e.g., `"spark 4"`).
104+
- Only add labels that already exist in the repo. If a label from the guide is
105+
missing in the repo, skip it for that issue and record a note in the summary
106+
rather than creating new labels.
107+
- Do not comment on the issue.
108+
109+
If `gh issue edit` fails for any issue, leave that issue's `requires-triage`
110+
label intact and record the failure in the summary under a "Failed to label"
111+
section.
112+
113+
## Step 6: File the Summary Issue
114+
115+
Compute today's date in `YYYY-MM-DD` form (use the system date, not memory):
116+
117+
```bash
118+
TRIAGE_DATE=$(date -u +%Y-%m-%d)
119+
```
120+
121+
Title: `Bug triage results: ${TRIAGE_DATE}`
122+
123+
Body: a markdown report with these sections, in this order:
124+
125+
1. **Header**
126+
- Date, total issues processed, and counts per priority
127+
- Link to `docs/source/contributor-guide/bug_triage.md`
128+
- Note that labels have already been applied; the reviewer should spot-check
129+
and close this issue when satisfied
130+
2. **Triaged** — one subsection per priority, ordered highest priority first
131+
(`priority:critical`, then `priority:high`, then `priority:medium`, then
132+
`priority:low`). Omit any subsection whose count is zero. Do **not** use a
133+
markdown table anywhere in this section; use nested bullet lists only.
134+
135+
Within each subsection, one top-level bullet per issue:
136+
137+
```
138+
### priority:critical
139+
140+
- <issue title> ([#1234](https://github.com/apache/datafusion-comet/issues/1234))
141+
- Area labels: `area:expressions`, `area:scan`
142+
- Rationale: one sentence tying the call to the guide
143+
```
144+
145+
The issue number (not the title) is the link target. The title is plain
146+
text. If there are no area labels, write `Area labels: none`.
147+
148+
3. **Escalations to consider** (omit section if empty) — bullet per issue with
149+
the same `<title> ([#N](url))` form, plus a sub-bullet explaining the
150+
trigger from the guide.
151+
4. **Skipped — needs more info** (omit if empty) — bullet per issue with the
152+
same `<title> ([#N](url))` form, plus a sub-bullet explaining what is
153+
missing.
154+
5. **Failed to label** (omit if empty) — bullet per issue with the same
155+
`<title> ([#N](url))` form, plus a sub-bullet quoting the `gh` error.
156+
157+
File the issue with `gh`. Use a temp file for the body to keep quoting sane:
158+
159+
```bash
160+
gh issue create \
161+
--repo apache/datafusion-comet \
162+
--title "Bug triage results: ${TRIAGE_DATE}" \
163+
--body-file /tmp/triage-summary-${TRIAGE_DATE}.md
164+
```
165+
166+
Do not add labels to the summary issue itself.
167+
168+
After creating the issue, print its URL.
169+
170+
## Output to the User
171+
172+
Report back:
173+
174+
1. Number of `requires-triage` issues processed
175+
2. Counts per priority that were applied
176+
3. Number skipped (needs more info) and number failed
177+
4. URL of the new summary issue
178+
179+
Do not paste the full per-issue listing back into the chat; it is in the
180+
summary issue.
181+
182+
## What This Skill Must Not Do
183+
184+
- Do not invent priority or area labels that are not in the guide
185+
- Do not create new labels in the repo
186+
- Do not comment on the triaged issues
187+
- Do not close any triaged issue
188+
- Do not file the summary issue if there were zero `requires-triage` issues
189+
- Do not re-label issues that were skipped or failed (leave `requires-triage`
190+
in place so they show up in the next pass)
191+
- Do not include AI/Claude attribution in the summary issue

‎.claude/skills/implement-comet-expression/SKILL.md‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
---
2+
name: implement-comet-expression
3+
description: Use when implementing a new Spark expression in DataFusion Comet. Walks through cloning latest Spark master to study the canonical implementation, checking the upstream datafusion-spark crate before writing native code, building the Comet serde and Rust wire-up from the contributor guide, then running audit-comet-expression to drive a test-coverage iteration loop.
4+
argument-hint: <expression-name>
5+
---
6+
7+
Implement Comet support for the `$ARGUMENTS` Spark expression.
8+
9+
## Background reading
10+
11+
The contributor guide is the canonical reference. Read these before writing code:
12+
13+
- `docs/source/contributor-guide/adding_a_new_expression.md` covers the Scala serde, protobuf, Rust scalar function flow, support levels, shims, and tests.
14+
- `docs/source/contributor-guide/sql-file-tests.md` describes the Comet SQL Tests format.
15+
- `docs/source/contributor-guide/spark_expressions_support.md` lists the coverage status for every expression.
16+
17+
## Workflow
18+
19+
### 1. Study the Spark master implementation first
20+
21+
Always start from the latest Spark `master`. Shallow clone if not already present:
22+
23+
```bash
24+
if [ ! -d /tmp/spark-master ]; then
25+
git clone --depth 1 https://github.com/apache/spark.git /tmp/spark-master
26+
fi
27+
```
28+
29+
Find the expression class and tests:
30+
31+
```bash
32+
find /tmp/spark-master/sql -name "*.scala" | \
33+
xargs grep -l "case class $ARGUMENTS\b\|object $ARGUMENTS\b" 2>/dev/null
34+
35+
find /tmp/spark-master/sql -name "*.scala" -path "*/test/*" | \
36+
xargs grep -l "$ARGUMENTS" 2>/dev/null
37+
```
38+
39+
Read the source. Note `inputTypes`, `dataType`, `eval` / `nullSafeEval`, ANSI mode branches, and any `require` guards. These define the contract Comet must match.
40+
41+
### 2. Check for an upstream `datafusion-spark` implementation
42+
43+
Before writing a Comet-specific native function, check whether the expression is already available in the upstream `datafusion-spark` crate. It is a Spark-compatible function library maintained alongside DataFusion, so its semantics are usually a closer match to Spark than a generic `datafusion-functions` built-in.
44+
45+
```bash
46+
grep -rn "fn name\|SparkFunctionName" ~/.cargo/registry/src/*/datafusion-spark-*/src/function/ 2>/dev/null | grep -i "$ARGUMENTS"
47+
```
48+
49+
Functions are organized as `datafusion_spark::function::<category>::<name>::Spark<Name>`. Existing wire-ups can be found in `native/core/src/execution/planner.rs` (e.g. `SparkDateAdd`, `SparkDateSub`, `SparkCollectSet`).
50+
51+
When the upstream implementation matches Spark's semantics, prefer it: register the `ScalarUDF` from `datafusion-spark` rather than re-implementing. This keeps the maintenance burden upstream. If the upstream version is missing, incomplete, or diverges from Spark, fall through to step 3 and write the function locally.
52+
53+
### 3. Implement the initial version
54+
55+
Follow `adding_a_new_expression.md`:
56+
57+
1. Add a `CometExpressionSerde[T]` in the appropriate file under `spark/src/main/scala/org/apache/comet/serde/`.
58+
2. Register it in the matching map in `QueryPlanSerde.scala`.
59+
3. If the function name collides with a DataFusion built-in that has a different signature, use `scalarFunctionExprToProtoWithReturnType` (see "When to set the return type explicitly").
60+
4. For a new scalar function, add a match case in `native/spark-expr/src/comet_scalar_funcs.rs::create_comet_physical_fun`. If step 2 found an upstream implementation, wire that in. Otherwise implement the function under `native/spark-expr/src/`.
61+
5. Add at least one Comet SQL Test at `spark/src/test/resources/sql-tests/expressions/<category>/$ARGUMENTS.sql` exercising column references, literals, and `NULL`.
62+
63+
Build and smoke-test:
64+
65+
```bash
66+
make
67+
./mvnw test -Dsuites="org.apache.comet.CometSqlFileTestSuite $ARGUMENTS" -Dtest=none
68+
```
69+
70+
### 4. Run the audit skill
71+
72+
Once the initial implementation passes its smoke test, run the `audit-comet-expression` skill on `$ARGUMENTS`. It compares the implementation and tests against Spark 3.4.3, 3.5.8, and 4.0.1 and produces a prioritized list of gaps.
73+
74+
### 5. Implement audit-recommended tests and iterate
75+
76+
Add the missing test cases the audit recommends, then re-run the targeted suite:
77+
78+
```bash
79+
./mvnw test -Dsuites="org.apache.comet.CometSqlFileTestSuite $ARGUMENTS" -Dtest=none
80+
```
81+
82+
Surface findings to the user and ask whether the coverage is sufficient. Continue iterating (adding tests, fixing bugs, refining `getSupportLevel` / `getIncompatibleReasons` / `getUnsupportedReasons`) until the user confirms they are happy.
83+
84+
### 6. Final checks
85+
86+
Before opening a PR:
87+
88+
```bash
89+
make format
90+
cd native && cargo clippy --all-targets --workspace -- -D warnings
91+
```
92+
93+
### 7. Open the PR
94+
95+
Use the repo's PR template at `.github/pull_request_template.md` and fill in every section: "Which issue does this PR close?", "Rationale for this change", "What changes are included in this PR?", and "How are these changes tested?". Do not add a separate test plan section.
96+
97+
In the "What changes are included in this PR?" section, add a brief note that the `implement-comet-expression` skill was used to scaffold the implementation, so reviewers know which workflow produced the change.

‎.github/actions/setup-spark-builder/action.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,23 @@ runs:
6767
run: |
6868
# Native library should already be in native/target/release/
6969
./mvnw install -Prelease -DskipTests -Pspark-${{inputs.spark-short-version}}
70+
71+
- name: Purge partial Maven cache entries
72+
shell: bash
73+
run: |
74+
# Comet's Maven phase resolves the dependency graph and downloads POMs
75+
# for transitive artifacts whose JARs it never actually needs. When sbt
76+
# then resolves Spark's deps, Coursier sees the POM in mavenLocal,
77+
# declares the artifact "found locally", and fails on the missing JAR
78+
# without falling back to Maven Central. Delete those partial entries
79+
# so sbt re-fetches the full artifact remotely.
80+
for repo in "$HOME/.m2/repository" /root/.m2/repository; do
81+
[ -d "$repo" ] || continue
82+
find "$repo" -name '*.pom' | while read -r pom; do
83+
jar="${pom%.pom}.jar"
84+
[ -f "$jar" ] && continue
85+
grep -q '<packaging>jar</packaging>\|<packaging>bundle</packaging>' "$pom" 2>/dev/null || continue
86+
rm -f "$pom" "${pom}.sha1" "${pom%.pom}.pom.lastUpdated" \
87+
"$(dirname "$pom")/_remote.repositories"
88+
done
89+
done

‎.github/workflows/iceberg_spark_test.yml‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,14 @@ jobs:
120120
strategy:
121121
matrix:
122122
os: [ubuntu-24.04]
123-
java-version: [11, 17]
124123
iceberg-version: [{short: '1.8', full: '1.8.1'}, {short: '1.9', full: '1.9.1'}, {short: '1.10', full: '1.10.0'}]
125124
spark-version: [{short: '3.4', full: '3.4.3'}, {short: '3.5', full: '3.5.8'}]
126125
scala-version: ['2.13']
126+
include:
127+
- spark-version: {short: '3.4', full: '3.4.3'}
128+
java-version: 11
129+
- spark-version: {short: '3.5', full: '3.5.8'}
130+
java-version: 17
127131
fail-fast: false
128132
name: iceberg-spark/${{ matrix.os }}/iceberg-${{ matrix.iceberg-version.full }}/spark-${{ matrix.spark-version.full }}/scala-${{ matrix.scala-version }}/java-${{ matrix.java-version }}
129133
runs-on: ${{ matrix.os }}
@@ -163,10 +167,14 @@ jobs:
163167
strategy:
164168
matrix:
165169
os: [ubuntu-24.04]
166-
java-version: [11, 17]
167170
iceberg-version: [{short: '1.8', full: '1.8.1'}, {short: '1.9', full: '1.9.1'}, {short: '1.10', full: '1.10.0'}]
168171
spark-version: [{short: '3.4', full: '3.4.3'}, {short: '3.5', full: '3.5.8'}]
169172
scala-version: ['2.13']
173+
include:
174+
- spark-version: {short: '3.4', full: '3.4.3'}
175+
java-version: 11
176+
- spark-version: {short: '3.5', full: '3.5.8'}
177+
java-version: 17
170178
fail-fast: false
171179
name: iceberg-spark-extensions/${{ matrix.os }}/iceberg-${{ matrix.iceberg-version.full }}/spark-${{ matrix.spark-version.full }}/scala-${{ matrix.scala-version }}/java-${{ matrix.java-version }}
172180
runs-on: ${{ matrix.os }}
@@ -206,10 +214,14 @@ jobs:
206214
strategy:
207215
matrix:
208216
os: [ubuntu-24.04]
209-
java-version: [11, 17]
210217
iceberg-version: [{short: '1.8', full: '1.8.1'}, {short: '1.9', full: '1.9.1'}, {short: '1.10', full: '1.10.0'}]
211218
spark-version: [{short: '3.4', full: '3.4.3'}, {short: '3.5', full: '3.5.8'}]
212219
scala-version: ['2.13']
220+
include:
221+
- spark-version: {short: '3.4', full: '3.4.3'}
222+
java-version: 11
223+
- spark-version: {short: '3.5', full: '3.5.8'}
224+
java-version: 17
213225
fail-fast: false
214226
name: iceberg-spark-runtime/${{ matrix.os }}/iceberg-${{ matrix.iceberg-version.full }}/spark-${{ matrix.spark-version.full }}/scala-${{ matrix.scala-version }}/java-${{ matrix.java-version }}
215227
runs-on: ${{ matrix.os }}

‎.github/workflows/pr_build_linux.yml‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,10 @@ jobs:
9797
- name: "Spark 4.0, JDK 21"
9898
java_version: "21"
9999
maven_opts: "-Pspark-4.0"
100-
# Spark 4.1 is intentionally absent: the lint job invokes -Psemanticdb,
101-
# but semanticdb-scalac_2.13.17 is not yet published, so we cannot
102-
# currently run scalafix against the spark-4.1 profile.
100+
# Spark 4.1 and 4.2 are intentionally absent: the lint job invokes -Psemanticdb,
101+
# but semanticdb-scalac for those Scala patch versions (2.13.17 / 2.13.18) is not
102+
# yet published, so we cannot currently run scalafix against the spark-4.1 or
103+
# spark-4.2 profiles.
103104
fail-fast: false
104105
steps:
105106
- uses: runs-on/action@742bf56072eb4845a0f94b3394673e4903c90ff0 # v2.1.0
@@ -305,6 +306,11 @@ jobs:
305306
java_version: "17"
306307
maven_opts: "-Pspark-4.1"
307308
scan_impl: "auto"
309+
310+
- name: "Spark 4.2, JDK 17"
311+
java_version: "17"
312+
maven_opts: "-Pspark-4.2"
313+
scan_impl: "auto"
308314
suite:
309315
- name: "fuzz"
310316
value: |
@@ -364,6 +370,7 @@ jobs:
364370
org.apache.spark.sql.comet.CometTaskMetricsSuite
365371
org.apache.spark.sql.comet.CometDppFallbackRepro3949Suite
366372
org.apache.spark.sql.comet.CometShuffleFallbackStickinessSuite
373+
org.apache.spark.sql.comet.CometDecimalArithmeticViewSuite
367374
org.apache.comet.objectstore.NativeConfigSuite
368375
- name: "expressions"
369376
value: |

0 commit comments

Comments
 (0)