Parquet, Arrow: Refactor vectorized reader #9772

wgtmac · 2024-02-22T06:22:09Z

This patch is the preparation work of #7162 and it contains following changes:

Add VectorizedValuesReader interface for future extension.
Refactor VectorizedPageIterator to use VectorizedValuesReader.
Consolidate duplicate code in the VectorizedPageIterator.

wgtmac · 2024-02-22T15:08:05Z

I plan to resolve #7162 by adding vectorized readers for all v2 encodings. This is the 1st patch. Would you mind taking a look? @rdblue @nastra @Fokko @jackye1995

anthonysgro · 2024-03-11T19:31:35Z

@wgtmac thanks so much for taking this issue on! I am just a user of iceberg (I have never contributed), but I was curious--is it also possible to include a resolution for #521 as an extension of this work? It would be a huge boon for iceberg performance since I often use complex/nested types, and I'm sure others do as well.

wgtmac · 2024-03-12T01:47:28Z

@anthonysgro Yes, that is on my radar. I will work on it once v2 encodings of all primitive types have been supported.

nastra · 2024-03-15T14:40:19Z

I plan to resolve #7162 by adding vectorized readers for all v2 encodings. This is the 1st patch.

@wgtmac do you by any chance have all other changes that are required for v2 encodings? I'd like to see and understand how the changes in this PR fit into the bigger picture (It's been a while since I touched/reviewed the vectorized code paths)

wgtmac · 2024-03-15T15:56:31Z

Not yet. My rough plan is to do following things:

add a new VectorizedValuesReader base class to supporting different encodings. This is similar to what spark does but reading into arrow field vector: :https://github.com/apache/spark/blob/b7aa9740249b50ad9db254626c530ff5bc33d385/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedValuesReader.java#L30
extend VectorizedValuesReader to add v2 encodings one by one.
support vectorized readers for nested types.

This patch is the step 1 above and only added vectorized reading interfaces for float/double/int32/int64 physical types. It already shows the big picture that how following steps will be done. More interfaces will be added progressively for reading other physical types and logical types into arrow field vectors in order to have a better review experience.

Does this make sense to you? @nastra

wgtmac

Please see my inline comments for the main changes. All new classes/interfaces are package-private and most of this PR is refactoring, so I think this approach is not aggressive. Hope this can help review the PR. @nastra

wgtmac · 2024-03-16T16:26:27Z

arrow/src/main/java/org/apache/iceberg/arrow/vectorized/parquet/VectorizedValuesReader.java

+import org.apache.parquet.io.api.Binary;
+
+/** Interface for value decoding that supports vectorized (aka batched) decoding. */
+interface VectorizedValuesReader {


This is the new package-private vectorized reader for a single page. All v2 encodings will be added by implementing this interface and selected readXXX() methods.

wgtmac · 2024-03-16T16:28:03Z

...java/org/apache/iceberg/arrow/vectorized/parquet/VectorizedParquetDefinitionLevelReader.java

@@ -47,35 +46,45 @@ public VectorizedParquetDefinitionLevelReader(
    super(bitWidth, maxDefLevel, readLength, setArrowValidityVector);
  }

-  abstract class NumericBaseReader {
+  abstract class BaseReader {


I have merged NumericBaseReader into BaseReader to eliminate the duplicate code. This refactoring contributes to the most part of this PR.

Perhaps I can split the refactoring work into a separate PR to make the review much easier.

wgtmac · 2024-03-16T16:29:44Z

...java/org/apache/iceberg/arrow/vectorized/parquet/VectorizedParquetDefinitionLevelReader.java

+        int typeWidth,
+        Mode mode);
+
+    protected void nextValues(


This method is where we utilize the vectorized reading method from VectorizedValuesReader. You can see LongReader/IntegerReader/FloatReader/DoubleReader have overridden this method.

wgtmac · 2024-03-16T16:30:34Z

arrow/src/main/java/org/apache/iceberg/arrow/vectorized/parquet/VectorizedPageIterator.java

@@ -46,7 +45,7 @@ public VectorizedPageIterator(
    this.setArrowValidityVector = setValidityVector;
  }

-  private ValuesAsBytesReader plainValuesReader = null;
+  private VectorizedValuesReader valuesReader = null;


I have replaced plainValuesReader with generic values reader to support different encodings.

wgtmac · 2024-03-16T16:31:36Z

arrow/src/main/java/org/apache/iceberg/arrow/vectorized/parquet/VectorizedPageIterator.java

-      if (dataEncoding != Encoding.PLAIN) {
+      if (dataEncoding == Encoding.PLAIN) {
+        valuesReader = new VectorizedPlainValuesReader();
+      } else {


More VectorizedValuesReader implementations will be added here for different encodings.

wgtmac · 2024-03-16T16:32:43Z

...java/org/apache/iceberg/arrow/vectorized/parquet/VectorizedParquetDefinitionLevelReader.java

+        int numValues,
+        VectorizedValuesReader valuesReader,
+        int typeWidth) {
+      valuesReader.readLongs(numValues, vector, bufferIdx);


For example, LongReader overrides nextValues to use VectorizedValuesReader.readLongs() method for vectorized reading.

- Add VectorizedValuesReader interface for future extension. - Refactor VectorizedPageIterator to use VectorizedValuesReader. - Consolidate duplicate code in the VectorizedPageIterator.

wgtmac · 2024-07-24T14:55:20Z

Since this feature has been requested in the community but the PR is not reviewed for a long time. Do you think this is the right direction here? Or should I directly support reading arrow vectors from the parquet-java library? @nastra @aokolnychyi @Fokko @szehon-ho

nastra · 2024-07-24T15:07:29Z

hey @wgtmac sorry for the long wait here but I don't have any spare cycles atm to review this.

github-actions · 2024-10-21T00:16:01Z

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

PaulLiang1 · 2024-10-21T00:25:10Z

Bump comment to keep the bot from closing this

flyrain · 2025-02-06T23:36:24Z

Hi @wgtmac , can we resume this PR? I can help with review.

github-actions bot added the arrow label Feb 22, 2024

wgtmac force-pushed the vectorized_plain_reader branch from 5a0fe0f to 8438906 Compare February 22, 2024 06:32

Fokko requested a review from aokolnychyi March 11, 2024 19:41

wgtmac force-pushed the vectorized_plain_reader branch from 08f4754 to 92ba90a Compare March 16, 2024 16:25

wgtmac commented Mar 16, 2024

View reviewed changes

Parquet, Arrow: Refactor vectorized reader

f47157d

- Add VectorizedValuesReader interface for future extension. - Refactor VectorizedPageIterator to use VectorizedValuesReader. - Consolidate duplicate code in the VectorizedPageIterator.

wgtmac force-pushed the vectorized_plain_reader branch from 92ba90a to f47157d Compare March 16, 2024 16:38

github-actions bot added the stale label Oct 21, 2024

nastra added not-stale and removed stale labels Oct 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parquet, Arrow: Refactor vectorized reader #9772

Parquet, Arrow: Refactor vectorized reader #9772

wgtmac commented Feb 22, 2024

wgtmac commented Feb 22, 2024

anthonysgro commented Mar 11, 2024

wgtmac commented Mar 12, 2024

nastra commented Mar 15, 2024

wgtmac commented Mar 15, 2024 •

edited

Loading

wgtmac left a comment

wgtmac Mar 16, 2024

wgtmac Mar 16, 2024 •

edited

Loading

wgtmac Mar 16, 2024

wgtmac Mar 16, 2024

wgtmac Mar 16, 2024

wgtmac Mar 16, 2024

wgtmac Mar 16, 2024

wgtmac commented Jul 24, 2024 •

edited

Loading

nastra commented Jul 24, 2024

github-actions bot commented Oct 21, 2024

PaulLiang1 commented Oct 21, 2024

flyrain commented Feb 6, 2025

Parquet, Arrow: Refactor vectorized reader #9772

Are you sure you want to change the base?

Parquet, Arrow: Refactor vectorized reader #9772

Conversation

wgtmac commented Feb 22, 2024

wgtmac commented Feb 22, 2024

anthonysgro commented Mar 11, 2024

wgtmac commented Mar 12, 2024

nastra commented Mar 15, 2024

wgtmac commented Mar 15, 2024 • edited Loading

wgtmac left a comment

Choose a reason for hiding this comment

wgtmac Mar 16, 2024

Choose a reason for hiding this comment

wgtmac Mar 16, 2024 • edited Loading

Choose a reason for hiding this comment

wgtmac Mar 16, 2024

Choose a reason for hiding this comment

wgtmac Mar 16, 2024

Choose a reason for hiding this comment

wgtmac Mar 16, 2024

Choose a reason for hiding this comment

wgtmac Mar 16, 2024

Choose a reason for hiding this comment

wgtmac Mar 16, 2024

Choose a reason for hiding this comment

wgtmac commented Jul 24, 2024 • edited Loading

nastra commented Jul 24, 2024

github-actions bot commented Oct 21, 2024

PaulLiang1 commented Oct 21, 2024

flyrain commented Feb 6, 2025

wgtmac commented Mar 15, 2024 •

edited

Loading

wgtmac Mar 16, 2024 •

edited

Loading

wgtmac commented Jul 24, 2024 •

edited

Loading