Fix writing of parquet bloom filters

[raunaqmorarka]

Description

Current logic was failing to convert dictionary pages into bloom filter
when the fallback from dictionary to plain encoding happened after first page.
Bloom filter writing logic is changed to always collect a bloom filter and
discard it at the end if only dictionary encoded pages are present. This avoids
the need to depend on values writer fallback mechanism.

Additional context and related issues

Fixes #22701

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Hive, Delta, Iceberg
* Fix writing of bloom filter columns in parquet files. ({issue}`22701`)

[sopel39]

Bloom filter writing logic is changed to always collect a bloom filter and
discard it at the end if dictonary is present

So dictionaries and bloom filters are exclusive? Is there a potential perf degradation?

Current logic was failing to convert dictionary pages into bloom filter
when the fallback from dictionary to plain encoding happened after first page.

Do we know the reason.

[jkylling]

The current logic assumed fallback would only happen on the first page/there would only be one page.

[raunaqmorarka]

So dictionaries and bloom filters are exclusive?

Initial PR was a bit too restrictive, i've tweaked it a bit to discard bloom filter only if all pages are dictionary encoded, rather than just on presence of dictionary.

Is there a potential perf degradation?

We will pay extra cost of building bloom filter even if the column turns out to be fully dictionary encoded. This will be only on bloom filter configured columns though.

The problem with the fallback based approach was that it would be more complicated to make ValueWriter remember all previous dictionary encoded pages and decode them back to plain values for populating into bloom filter. It's simpler to just populate bloom filter separately and discard if it's redundant with dictionary.

[jkylling]

Thank you!

[sopel39]

Why we can't have both always? Seems like we should be able to perform bloom filtering even in the presence of dictionaries?

[raunaqmorarka]

For fully dictionary encoded columns, the reader is already able to perform row-group pruning based on dictionary entries. Having a bloom filter doesn't give us anything extra.

[sopel39]

For fully dictionary encoded columns, the reader is already able to perform row-group pruning based on dictionary entries

I thought that bloom filters could be more efficient at filtering for tactical (e.g. single value search) queries.

[raunaqmorarka]

It's lookup in a set vs lookup in a bloom filter. The CPU difference will be barely noticeable compared to everything else that goes on in parquet reader. Reading bloom filter takes extra reads from file and potentially has false positives.