andygrove opened a new pull request, #2565:
URL: https://github.com/apache/datafusion-ballista/pull/2565

   # Which issue does this PR close?
   
   No issue was filed.
   
   # Rationale for this change
   
   The `tpcds` binary is a correctness gate. It runs each query once, prints 
the time, and writes nothing to disk, so it can't be used to benchmark TPC-DS 
the way `tpch benchmark ballista` benchmarks TPC-H. It also doesn't fit TPC-DS 
data generated by other tools:
   
   - It always rewrites three column names to the ones `tpcgen-cli` uses 
(`ib_income_band_id`, `r_reason_description`, `cr_return_amount_inc_tax`). Data 
from `dsdgen` or Spark uses the spec names that the queries already use, so the 
rewrite breaks q64, q81, q84, q85 and q93 on that data.
   - It registers tables without their Hive partition columns. Data written 
with Spark's `partitionBy` only stores the fact tables' date keys 
(`ss_sold_date_sk` and so on) in the directory names, so those columns would be 
missing.
   
   Much of what a benchmark run needs already exists in `tpch.rs`: iterations, 
the JSON summary written after every query, per-query error recording, config 
overrides, and partition-aware Parquet registration. This PR moves those pieces 
into the `ballista-benchmarks` library and builds the TPC-DS benchmark on them, 
so the two runners share the code rather than each having a copy.
   
   # What changes are included in this PR?
   
   - **Shared code** (`benchmarks/src/lib.rs`, new `benchmarks/src/summary.rs`):
     - `summary::{BenchmarkRun, QueryRun, QueryResult}`, which are unchanged, 
and `run_suite`. `run_suite` runs the queries in order, records each one's 
iterations or error, writes `<name>-<start_time>.json` atomically after every 
query, prints the suite total and a list of failures, and returns an error if 
any query failed. `tpch benchmark ballista`, `tpch benchmark datafusion` and 
`tpcds` all use it.
     - `ballista_context` builds the per-query Ballista session: S3 support, 
target partitions, job name, batch size and `-c key=value` overrides.
     - `register_parquet_table` and `parquet_table_layout` move out of 
`tpch.rs`. They now take a `PathColumnType` function that gives the type of a 
partition column that only exists in the directory names. For TPC-H that type 
comes from the TPC-H schema, as before. `register_parquet_tables` uses them 
too, so the `tpcds` runner and its `--verify` oracle also declare partition 
columns.
   - **`tpcds` runner:**
     - New `--iterations`, `--output` and `--no-partition-cols` options, which 
mean what they mean for `tpch`. The summary uses the TPC-H format, so `tpch 
compare` reads it.
     - Column renames now apply only when the registered tables have the 
`tpcgen-cli` name and not the spec name. CI's `tpcgen-cli` data still gets all 
three.
     - A partition column that only exists in the directory names is read as 
`Int32` if it is a surrogate key (`*_sk`), because TPC-DS partitions its fact 
tables by date key.
     - Uses mimalloc, like `tpch`.
     - The CLI is otherwise unchanged, so the CI invocation still works.
   - **`tpch` runner:** no change in behaviour, with one exception. A query 
file that can't be loaded is now recorded as that query's failure instead of 
ending the run.
   - **Tests:** the `QueryRun` tests move with the struct. New tests cover 
`run_suite` (failures are recorded and the suite carries on, the summary file 
name and contents, and no output directory), which renames apply to 
`tpcgen-cli` and to spec column names, and a Spark-style 
`store_sales/ss_sold_date_sk=N/` table that registers the key as `Int32` and 
scans only the matching partition.
   - **Docs:** the TPC-DS section of `benchmarks/README.md` describes the 
benchmark mode, partition handling and the conditional renames.
   
   # Are there any user-facing changes?
   
   `tpcds` gains `--iterations`, `--output` and `--no-partition-cols`. It also 
declares Hive partition columns by default, and it no longer rewrites column 
names on data that uses the spec names. Ballista's own APIs don't change.
   


-- 
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]

Reply via email to