Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Jun 13, 2024

Which issue does this PR close?

Closes #.

Rationale for this change

While reviewing #10894 I noticed the tpch data generator seems broken


andrewlamb@Andrews-MacBook-Pro-2:~/Software/datafusion$ ./benchmarks/bench.sh data
...

warning: field `range` is never read
   --> datafusion/physical-expr/src/window/window_expr.rs:562:9
    |
561 | pub struct NthValueState {
    |            ------------- field in this struct
562 |     pub range: Range<usize>,
    |         ^^^^^
    |
    = note: `NthValueState` has derived impls for the traits `Clone` and `Debug`, but these are intentionally ignored during dead code analysis
    = note: `#[warn(dead_code)]` on by default

warning: `datafusion-physical-expr` (lib) generated 1 warning
    Finished `release-nonlto` profile [optimized] target(s) in 0.10s
     Running `/Users/andrewlamb/Software/datafusion/target/release-nonlto/tpch convert --input /Users/andrewlamb/Software/datafusion/benchmarks/data/tpch_sf1 --prefer_hash_join true --output /Users/andrewlamb/Software/datafusion/benchmarks/data/tpch_sf1 --format parquet`
error: Found argument '--prefer_hash_join' which wasn't expected, or isn't valid in this context

What changes are included in this PR?

fix the script

Are these changes tested?

Are there any user-facing changes?

echo " creating parquet files using benchmark binary ..."
pushd "${SCRIPT_DIR}" > /dev/null
$CARGO_COMMAND --bin tpch -- convert --input "${TPCH_DIR}" --prefer_hash_join ${PREFER_HASH_JOIN} --output "${TPCH_DIR}" --format parquet
$CARGO_COMMAND --bin tpch -- convert --input "${TPCH_DIR}" --output "${TPCH_DIR}" --format parquet
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the convert command doesn't take the --prefer_hash_join .. option

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, it was my bad

Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm thanks @alamb

@comphead comphead merged commit 4dd4121 into apache:main Jun 14, 2024
@alamb alamb deleted the alamb/fix_bench branch June 14, 2024 23:18
findepi pushed a commit to findepi/datafusion that referenced this pull request Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants