-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Enable bloom filters by default on read #10299
Comments
Thank you @hiltontj -- this is a great description. I agree it seems like a good idea to enable using bloom filters on read when they are present in the file. The test coverage is pretty good as far as I know these and I believe several users are already using the bloom filters @hengfeiyang -- as you contributed the initial implementation in #7821, do you know of any reason we shouldn't enable bloom filters by default? @Ted-Jiang or @progval -- I think you have worked on this feature recently as well -- do you know of any reason we shouldn't change the default? |
(BTW this is a wonderfully clear ticket in my mind -- thank you @hiltontj ) |
@alamb As an experiment feature, we disable it as default on first implementation. But i think we can enable this feature by default now. we already enabled as default in our production. |
I don't see any reason not to enable them. Ditto with |
I created a pr to split parquet bloom filter config and enable bloom filter on read by default #10306. |
Is your feature request related to a problem or challenge?
When reading from parquet files, bloom filters are not enabled by default. It is not immediately obvious that they are not being used when performing queries, so there may be users out there who are not aware that bloom filters in their parquet files are being ignored.
Part of the issue, however, is that the default behaviour looks to be shared between read and write operations.
Describe the solution you'd like
It would be ideal if bloom filters were enabled by default on read. We should be careful, however, as I do not think they should be enabled by default on write, where, depending on how they are configured, their inclusion can be expensive.
Describe alternatives you've considered
Currently, the bloom filters can be enabled, but must be done explicitly. For example, with
datafusion-cli
, which uses the default configuration, one must enable the setting via the environment, e.g.,or by setting it explicity, e.g.,
This may not work for everyone, however, since it may cause problems by writing with bloom filters enabled.
Additional context
Bloom filters are disabled by default here: https://github.com/apache/datafusion/blob/37.1.0/datafusion/common/src/config.rs#L398-L399
This setting is ultimately used to prune row groups on read here: https://github.com/apache/datafusion/blob/37.1.0/datafusion/core/src/datasource/physical_plan/parquet/mod.rs#L531-L545
It looks like this setting is also applied on write here: https://github.com/apache/datafusion/blob/37.1.0/datafusion/common/src/file_options/parquet_writer.rs#L68
There is an existing SLT test that explicitly enables this setting when performing a query here: https://github.com/apache/datafusion/blob/main/datafusion/sqllogictest/test_files/predicates.slt#L509-L547, however, I do not see any tests that are using this setting on write.
The text was updated successfully, but these errors were encountered: