You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "alliasgher (via GitHub)" <gi...@apache.org> on 2026/04/13 13:45:13 UTC
[PR] feat(table): wire bloom filter properties into Parquet writer [iceberg-go]
alliasgher opened a new pull request, #878:
URL: https://github.com/apache/iceberg-go/pull/878
## Summary
`GetWriteProperties` read compression, page size, row group size etc. but silently ignored the two bloom filter properties defined in the same file:
- `write.parquet.bloom-filter-max-bytes` — caps the per-column bloom filter size (default 1 MB)
- `write.parquet.bloom-filter-enabled.column.<name>` — enables bloom filters per column
Wire them through to the Arrow Parquet writer:
- `WithMaxBloomFilterBytes` is always applied (defaults to the existing `ParquetBloomFilterMaxBytesDefault` constant).
- All table properties whose key starts with `write.parquet.bloom-filter-enabled.column.` are scanned; for each match `WithBloomFilterEnabledFor` is called with the column name and parsed boolean.
Fixes #844
## Test plan
- [x] `go test ./...`
- [x] `gofmt` clean
- [x] New `TestGetWritePropertiesBloomFilter` covering default max-bytes, custom max-bytes, and per-column enable/disable.
Signed-off-by: Ali <al...@gmail.com>
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(table): wire bloom filter properties into Parquet writer [iceberg-go]
Posted by "alliasgher (via GitHub)" <gi...@apache.org>.
alliasgher commented on code in PR #878:
URL: https://github.com/apache/iceberg-go/pull/878#discussion_r3074672595
##########
table/internal/parquet_files.go:
##########
@@ -259,8 +259,27 @@ func (parquetFormat) GetWriteProperties(props iceberg.Properties) any {
slog.Warn("unrecognized compression codec, falling back to uncompressed", "codec", compression)
}
- return append(writerProps, parquet.WithCompression(codec),
+ writerProps = append(writerProps, parquet.WithCompression(codec),
parquet.WithCompressionLevel(compressionLevel))
+
+ // Bloom filter properties.
+ // write.parquet.bloom-filter-max-bytes caps the per-column bloom filter size.
+ bloomMaxBytes := props.GetInt(ParquetBloomFilterMaxBytesKey, ParquetBloomFilterMaxBytesDefault)
+ writerProps = append(writerProps, parquet.WithMaxBloomFilterBytes(int64(bloomMaxBytes)))
+
+ // write.parquet.bloom-filter-enabled.column.<col-name> enables bloom filters
+ // for individual columns. Scan all properties for the prefix.
+ prefix := ParquetBloomFilterColumnEnabledKeyPrefix + "."
+ for key, val := range props {
+ colName, ok := strings.CutPrefix(key, prefix)
+ if !ok || colName == "" {
+ continue
+ }
+ enabled := strings.EqualFold(val, "true")
Review Comment:
Added a comment in 9684d10 explaining this.
##########
table/internal/parquet_files.go:
##########
@@ -259,8 +259,27 @@ func (parquetFormat) GetWriteProperties(props iceberg.Properties) any {
slog.Warn("unrecognized compression codec, falling back to uncompressed", "codec", compression)
}
- return append(writerProps, parquet.WithCompression(codec),
+ writerProps = append(writerProps, parquet.WithCompression(codec),
Review Comment:
Agreed, harmless as-is — leaving it unconditional for now since the parquet writer ignores it when no bloom filters are written.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(table): wire bloom filter properties into Parquet writer [iceberg-go]
Posted by "alliasgher (via GitHub)" <gi...@apache.org>.
alliasgher commented on code in PR #878:
URL: https://github.com/apache/iceberg-go/pull/878#discussion_r3074673082
##########
table/internal/parquet_files_test.go:
##########
@@ -691,6 +691,33 @@ func TestGetWritePropertiesPageVersion(t *testing.T) {
}
}
+func TestGetWritePropertiesBloomFilter(t *testing.T) {
+ format := internal.GetFileFormat(iceberg.ParquetFile)
+
+ t.Run("default max bloom filter bytes", func(t *testing.T) {
+ wp := parquet.NewWriterProperties(format.GetWriteProperties(iceberg.Properties{}).([]parquet.WriterProperty)...)
+ assert.Equal(t, int64(internal.ParquetBloomFilterMaxBytesDefault), wp.MaxBloomFilterBytes())
+ })
+
+ t.Run("custom max bloom filter bytes", func(t *testing.T) {
+ props := iceberg.Properties{
+ internal.ParquetBloomFilterMaxBytesKey: "2097152",
+ }
+ wp := parquet.NewWriterProperties(format.GetWriteProperties(props).([]parquet.WriterProperty)...)
+ assert.Equal(t, int64(2097152), wp.MaxBloomFilterBytes())
+ })
+
+ t.Run("per-column bloom filter enabled", func(t *testing.T) {
+ props := iceberg.Properties{
+ internal.ParquetBloomFilterColumnEnabledKeyPrefix + ".id": "true",
+ internal.ParquetBloomFilterColumnEnabledKeyPrefix + ".name": "false",
+ }
+ wp := parquet.NewWriterProperties(format.GetWriteProperties(props).([]parquet.WriterProperty)...)
+ assert.True(t, wp.BloomFilterEnabledFor("id"))
Review Comment:
Added in 9684d10.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(table): wire bloom filter properties into Parquet writer [iceberg-go]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on code in PR #878:
URL: https://github.com/apache/iceberg-go/pull/878#discussion_r3074279308
##########
table/internal/parquet_files.go:
##########
@@ -259,8 +259,27 @@ func (parquetFormat) GetWriteProperties(props iceberg.Properties) any {
slog.Warn("unrecognized compression codec, falling back to uncompressed", "codec", compression)
}
- return append(writerProps, parquet.WithCompression(codec),
+ writerProps = append(writerProps, parquet.WithCompression(codec),
parquet.WithCompressionLevel(compressionLevel))
+
+ // Bloom filter properties.
+ // write.parquet.bloom-filter-max-bytes caps the per-column bloom filter size.
+ bloomMaxBytes := props.GetInt(ParquetBloomFilterMaxBytesKey, ParquetBloomFilterMaxBytesDefault)
+ writerProps = append(writerProps, parquet.WithMaxBloomFilterBytes(int64(bloomMaxBytes)))
+
+ // write.parquet.bloom-filter-enabled.column.<col-name> enables bloom filters
+ // for individual columns. Scan all properties for the prefix.
+ prefix := ParquetBloomFilterColumnEnabledKeyPrefix + "."
+ for key, val := range props {
+ colName, ok := strings.CutPrefix(key, prefix)
+ if !ok || colName == "" {
+ continue
+ }
+ enabled := strings.EqualFold(val, "true")
Review Comment:
This means "false", "0", "yes", "1", or any typo all silently map to false.
Matches Java's Boolean.parseBoolean, so it's **correct**.
But worth a one-line comment since Go developers would expect strconv.ParseBool here, and the difference is real (strconv.ParseBool("1") is true).
##########
table/internal/parquet_files_test.go:
##########
@@ -691,6 +691,33 @@ func TestGetWritePropertiesPageVersion(t *testing.T) {
}
}
+func TestGetWritePropertiesBloomFilter(t *testing.T) {
+ format := internal.GetFileFormat(iceberg.ParquetFile)
+
+ t.Run("default max bloom filter bytes", func(t *testing.T) {
+ wp := parquet.NewWriterProperties(format.GetWriteProperties(iceberg.Properties{}).([]parquet.WriterProperty)...)
+ assert.Equal(t, int64(internal.ParquetBloomFilterMaxBytesDefault), wp.MaxBloomFilterBytes())
+ })
+
+ t.Run("custom max bloom filter bytes", func(t *testing.T) {
+ props := iceberg.Properties{
+ internal.ParquetBloomFilterMaxBytesKey: "2097152",
+ }
+ wp := parquet.NewWriterProperties(format.GetWriteProperties(props).([]parquet.WriterProperty)...)
+ assert.Equal(t, int64(2097152), wp.MaxBloomFilterBytes())
+ })
+
+ t.Run("per-column bloom filter enabled", func(t *testing.T) {
+ props := iceberg.Properties{
+ internal.ParquetBloomFilterColumnEnabledKeyPrefix + ".id": "true",
+ internal.ParquetBloomFilterColumnEnabledKeyPrefix + ".name": "false",
+ }
+ wp := parquet.NewWriterProperties(format.GetWriteProperties(props).([]parquet.WriterProperty)...)
+ assert.True(t, wp.BloomFilterEnabledFor("id"))
Review Comment:
Also assert that a column NOT mentioned in properties returns false (the parquet default):
assert.False(t, wp.BloomFilterEnabledFor("unmentioned_col"))
This pins the "no property = no bloom filter" contract, which is the important default behavior.
##########
table/internal/parquet_files.go:
##########
@@ -259,8 +259,27 @@ func (parquetFormat) GetWriteProperties(props iceberg.Properties) any {
slog.Warn("unrecognized compression codec, falling back to uncompressed", "codec", compression)
}
- return append(writerProps, parquet.WithCompression(codec),
+ writerProps = append(writerProps, parquet.WithCompression(codec),
Review Comment:
Nit: this is set unconditionally even when no bloom filters are enabled for any column. Harmless since parquet ignores it when no bloom filters are written, but slightly noisy. Not blocking.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(table): wire bloom filter properties into Parquet writer [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #878:
URL: https://github.com/apache/iceberg-go/pull/878
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org