andygrove opened a new pull request, #2183: URL: https://github.com/apache/datafusion-ballista/pull/2183
# Which issue does this PR close? Closes #. # Rationale for this change Five TPC-DS queries were excluded from the correctness gate as "tpcgen-cli schema column-name mismatch with query text". Checking the generator's schema against the query text, the mismatch turns out to be three pure renames — `tpcgen-cli` spells three columns differently from the TPC-DS spec, and the vendored DataFusion query text follows the spec: | Query text (TPC-DS spec) | tpcgen-cli emits | Affects | |---|---|---| | `ib_income_band_sk` | `ib_income_band_id` | q64, q84 | | `r_reason_desc` | `r_reason_description` | q85, q93 | | `cr_return_amt_inc_tax` | `cr_return_amount_inc_tax` | q81 | No column is missing — the data is present under the other name — so all five queries can run once the reference is rewritten. This takes the gate from 92 to 97 of 99 queries; only the two genuinely non-deterministic ones (q31, q71) remain skipped. Separately, a TPC-DS run that hangs currently sits until GitHub's 6-hour default before reporting anything. That is not hypothetical: a hung run was observed sitting for over three hours against ~7 minutes for a passing run. # What changes are included in this PR? **Query loading (`benchmarks/src/bin/tpcds.rs`)** - Adds a `COLUMN_RENAMES` table applied to the query text at load time, and removes q64, q81, q84, q85 and q93 from `SKIP`. - The rewrite is applied to the query string, so the Ballista run and the single-process DataFusion oracle see identical SQL and the comparison stays apples-to-apples. - It is applied at load time rather than by editing the files under `benchmarks/queries-tpcds/`, because `dev/vendor-tpcds-queries.sh` re-downloads all 99 queries and would clobber a local edit. - The replacement is whole-word rather than a plain `str::replace`: `r_reason_desc` is a prefix of `r_reason_description`, so a substring replace would corrupt an already correct reference and would not be idempotent. **Workflow (`.github/workflows/tpcds.yml`)** - Adds `timeout-minutes: 30` to the query step and a 45-minute job-level backstop. The primary timeout is on the *step* rather than the job so a hang still reaches the log-upload step and leaves something to diagnose. - Widens the log-upload condition from `failure()` to `!success()`, so a timed-out or cancelled run still uploads its scheduler/executor logs — the hang case is exactly when they matter most. - Drops the stale comment about #2047, which is fixed. # Are there any user-facing changes? No. This only affects CI coverage and the benchmark runner. ## Testing Four unit tests cover the word-boundary behaviour, idempotency, full coverage of the rename table, and that the rewrite is actually reached from the query load path. The full suite was run in CI on the stacked branch for #2182: the **AQE off / 16 partitions** configuration passed with all five newly-unskipped queries included, confirming they now execute and match the DataFusion oracle. -- 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]
