Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parquet benchmark is out of sync with the current code base #770

Closed
houqp opened this issue Sep 13, 2021 · 3 comments
Closed

parquet benchmark is out of sync with the current code base #770

houqp opened this issue Sep 13, 2021 · 3 comments
Labels

Comments

@houqp
Copy link
Member

houqp commented Sep 13, 2021

Describe the bug

parquet benchmark run is broken with the following error message:

error[E0432]: unresolved imports `parquet::util::DataPageBuilder`, `parquet::util::DataPageBuilderImpl`, `parquet::util::InMemoryPageIterator`
  --> parquet/benches/arrow_array_reader.rs:19:21
   |
19 | use parquet::util::{DataPageBuilder, DataPageBuilderImpl, InMemoryPageIterator};
   |                     ^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^^^^ no `InMemoryPageIterator` in `util`
   |                     |                |
   |                     |                no `DataPageBuilderImpl` in `util`
   |                     no `DataPageBuilder` in `util`

For more information about this error, try `rustc --explain E0432`.
error: could not compile `parquet` due to previous error
warning: build failed, waiting for other jobs to finish...
error: build failed

To Reproduce

run cargo bench

Expected behavior

benchmark should compile without error.

Additional context

would be good if we test for benchmark builds in CI by running cargo bench --no-run

@houqp houqp added the bug label Sep 13, 2021
@carols10cents
Copy link
Contributor

I just ran into this too. Looks like these types are now behind the test_common feature; running cargo check --benches --features=test_common -p parquet does compile.

I'm not sure if it would be ok to remove the #[cfg(any(test, feature = "test_common"))] for these types, or if running parquet benches should be documented to need the test_common features and tested in CI. I'm going to open a PR for the latter two, at least.

@alamb
Copy link
Contributor

alamb commented Jan 13, 2022

I think this was fixed in #1001

I now run cargo bench and it compiles just fine for me locally

@alamb alamb closed this as completed Jan 13, 2022
@alamb
Copy link
Contributor

alamb commented Jan 13, 2022

Thanks again @carols10cents

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants