[
https://issues.apache.org/jira/browse/PARQUET-2373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17782957#comment-17782957
]
ASF GitHub Bot commented on PARQUET-2373:
-----------------------------------------
wgtmac commented on code in PR #1184:
URL: https://github.com/apache/parquet-mr/pull/1184#discussion_r1382523507
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1347,11 +1348,24 @@ public BloomFilter readBloomFilter(ColumnChunkMetaData
meta) throws IOException
}
}
- // Read Bloom filter data header.
+ // Seek to Bloom filter offset.
f.seek(bloomFilterOffset);
+
+ // Read Bloom filter length.
+ int bloomFilterLength = meta.getBloomFilterLength();
+
+ // If it is set, read Bloom filter header and bitset together.
+ // Otherwise, read Bloom filter header first and then bitset.
+ InputStream in = null;
+ if (bloomFilterLength > 0) {
+ byte[] headerAndBitSet = new byte[bloomFilterLength];
+ f.readFully(headerAndBitSet);
+ in = new ByteArrayInputStream(headerAndBitSet);
+ }
+
BloomFilterHeader bloomFilterHeader;
try {
- bloomFilterHeader = Util.readBloomFilterHeader(f, bloomFilterDecryptor,
bloomFilterHeaderAAD);
+ bloomFilterHeader = Util.readBloomFilterHeader(in != null ? in : f,
bloomFilterDecryptor, bloomFilterHeaderAAD);
Review Comment:
Should we separate these two cases into two methods? Here and below have
several checks like `in != null ? in : f`.
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/ColumnChunkMetaData.java:
##########
@@ -341,6 +351,15 @@ public long getBloomFilterOffset() {
return bloomFilterOffset;
}
+ /**
+ * @return the length to the Bloom filter or {@code -1} if there is no bloom
filter for this column chunk
Review Comment:
```suggestion
* @return the length to the Bloom filter or {@code -1} if there is no
bloom filter length for this column chunk
```
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/ColumnChunkMetaData.java:
##########
@@ -341,6 +351,15 @@ public long getBloomFilterOffset() {
return bloomFilterOffset;
}
+ /**
+ * @return the length to the Bloom filter or {@code -1} if there is no bloom
filter for this column chunk
+ */
+ @Private
+ public int getBloomFilterLength() {
Review Comment:
BTW, an e2e test case will be helpful to guarantee we have written the
length correctly.
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/ColumnChunkMetaData.java:
##########
@@ -341,6 +351,15 @@ public long getBloomFilterOffset() {
return bloomFilterOffset;
}
+ /**
+ * @return the length to the Bloom filter or {@code -1} if there is no bloom
filter for this column chunk
+ */
+ @Private
+ public int getBloomFilterLength() {
Review Comment:
Return Optional<Int>?
> Improve I/O performance with bloom_filter_length
> ------------------------------------------------
>
> Key: PARQUET-2373
> URL: https://issues.apache.org/jira/browse/PARQUET-2373
> Project: Parquet
> Issue Type: Improvement
> Reporter: Jiashen Zhang
> Priority: Minor
>
> The spec PARQUET-2257 has added bloom_filter_length for reader to load the
> bloom filter in a single shot. This implementation alters the code to make
> use of the 'bloom_filter_length' field for loading the bloom filter
> (consisting of the header and bitset) in order to enhance I/O scheduling.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
