[HUDI-3396] Make sure `BaseFileOnlyViewRelation` only reads projected columns #4818

alexeykudinkin · 2022-02-15T02:16:12Z

Tips

Thank you very much for contributing to Apache Hudi.
Please review https://hudi.apache.org/contribute/how-to-contribute before opening a pull request.

What is the purpose of the pull request

NOTE: This change is first part of the series to clean up Hudi's Spark DataSource related implementations, making sure there's minimal code duplication among them, implementations are consistent and performant

This PR is making sure that BaseFileOnlyViewRelation only reads projected columns as well as avoiding unnecessary serde from Row to InternalRow

Brief change log

Introduced HoodieBaseRDD as a base for all custom RDD impls
Extracted common fields/methods to HoodieBaseRelation
Cleaned up and streamlined HoodieBaseFileViewOnlyRelation
Fixed all of the Relations to avoid superfluous Row <> InternalRow conversions

Verify this pull request

This pull request is already covered by existing tests, such as (please describe tests).

Committer checklist

Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green
Necessary doc changes done or have another open PR
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

alexeykudinkin · 2022-02-19T03:32:55Z

@hudi-bot run azure

nsivabalan

Good job on the patch!
mostly minor comments and some clarifications.

nsivabalan · 2022-02-26T00:02:58Z

...i-spark-client/src/test/java/org/apache/hudi/testutils/SparkClientFunctionalTestHarness.java

@@ -348,6 +355,21 @@ protected HoodieWriteConfig getConfig(Boolean autoCommit, Boolean rollbackUsingM
        .withRollbackUsingMarkers(rollbackUsingMarkers);
  }

+  protected Dataset<Row> toDataset(List<HoodieRecord> records) {


can we take in AvroSchema as an argument. may be create another overloaded method which calls into this w/ some default for avro schema.

is this addressed ?

...k-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/BaseFileOnlyViewRelation.scala

...ark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieDataSourceHelper.scala

hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieFileScanRDD.scala

hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieUnsafeRDD.scala

hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieFileScanRDD.scala

...source/hudi-spark-common/src/main/scala/org/apache/hudi/MergeOnReadIncrementalRelation.scala

...ource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestParquetColumnProjection.scala

…ible

…lemented appropriately

…nversions

Added java-docs elaborating the scope and purpose

…upon querying

Moving Spark version-specific logic into version-specific impls; Re-aligned components naming

alexeykudinkin · 2022-03-05T21:52:06Z

@hudi-bot run azure

...i-spark-client/src/test/java/org/apache/hudi/testutils/SparkClientFunctionalTestHarness.java

nsivabalan · 2022-03-06T20:09:40Z

...i-spark-client/src/test/java/org/apache/hudi/testutils/SparkClientFunctionalTestHarness.java

@@ -348,6 +355,21 @@ protected HoodieWriteConfig getConfig(Boolean autoCommit, Boolean rollbackUsingM
        .withRollbackUsingMarkers(rollbackUsingMarkers);
  }

+  protected Dataset<Row> toDataset(List<HoodieRecord> records) {


is this addressed ?

nsivabalan

left couple of comments

alexeykudinkin · 2022-03-08T00:55:59Z

@hudi-bot run azure

hudi-bot · 2022-03-08T02:04:28Z

CI report:

448555f Azure: FAILURE Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

nsivabalan

LGTM. But I will test this patch out and report the results. since this touches core read path, wanted to ensure things are ok.

… columns (apache#4818) NOTE: This change is first part of the series to clean up Hudi's Spark DataSource related implementations, making sure there's minimal code duplication among them, implementations are consistent and performant This PR is making sure that BaseFileOnlyViewRelation only reads projected columns as well as avoiding unnecessary serde from Row to InternalRow Brief change log - Introduced HoodieBaseRDD as a base for all custom RDD impls - Extracted common fields/methods to HoodieBaseRelation - Cleaned up and streamlined HoodieBaseFileViewOnlyRelation - Fixed all of the Relations to avoid superfluous Row <> InternalRow conversions

alexeykudinkin changed the title ~~[WIP][HUDI-3396] Make sure BaseFileOnlyViewRelation only reads projected columns~~ [WIP][HUDI-3396][Stacked on 4789] Make sure BaseFileOnlyViewRelation only reads projected columns Feb 15, 2022

nsivabalan assigned nsivabalan and yihua Feb 16, 2022

alexeykudinkin force-pushed the ak/spkds-ref-1 branch 2 times, most recently from 37d994c to acfa5bc Compare February 19, 2022 00:56

alexeykudinkin force-pushed the ak/spkds-ref-1 branch from acfa5bc to 4f0f5a2 Compare February 23, 2022 03:37

This was referenced Feb 23, 2022

[HUDI-3457] Refactored Spark DataSource Relations to avoid code duplication #4877

Merged

[HUDI-1296] Support Metadata Table in Spark Datasource #4789

Merged

alexeykudinkin force-pushed the ak/spkds-ref-1 branch from 4f0f5a2 to 2e19472 Compare February 24, 2022 21:30

alexeykudinkin changed the title ~~[WIP][HUDI-3396][Stacked on 4789] Make sure BaseFileOnlyViewRelation only reads projected columns~~ [HUDI-3396] Make sure BaseFileOnlyViewRelation only reads projected columns Feb 24, 2022

nsivabalan reviewed Feb 26, 2022

View reviewed changes

Alexey Kudinkin added 16 commits March 3, 2022 22:14

Refactored HoodieTestDataGenerator to make data generation reproduc…

4d582cf

…ible

Extracted common fields to HoodieBaseRelation;

f612d96

Cleaned up BaseFileOnlyViewRelation

4662f7d

Streamlined BaseFileOnlyViewRelation

46c56ef

Introduced HoodieBaseRDD to make sure all Hudi custom impls are imp…

2449ea1

…lemented appropriately

Fixed all of the Relations to avoid superfluous Row <> InternalRow co…

8f466d1

…nversions

Tidying up

ef42921

Fixing compilation

f12aa31

Fixing compilation after rebase

4d2d93b

After rebase fixes

ad4a142

Fixing compilation

fedd8ef

HoodieBaseRDD > HoodieUnsafeRDD;

c4bbe3f

Added java-docs elaborating the scope and purpose

HoodieBaseRDD > HoodieUnsafeRDD;

0b3079c

Added java-docs elaborating the scope and purpose

Bootstrapped HoodieUnsafeRDDUtils

c24d7d2

Missing license

a6260fd

Bootstrapped test for MOR table validating proper column projections …

2ec56ef

…upon querying

Alexey Kudinkin added 9 commits March 3, 2022 22:14

Broken down into multiple tests

f8f3ec2

Added test for COW

ac2684a

Added test for MOR Incremental Relation

b0218c5

Expose mandatoryColumns to be accessible in tests

edf7c32

Typo

21abebc

HoodieAvroSerializerTrait > HoodieAvroSerializer;

d6b2b6e

Moving Spark version-specific logic into version-specific impls; Re-aligned components naming

lint

c81f822

Added SparkHadoopUtil delegating to Spark's private impl

62edf7d

Fixed tests for Spark 2.x

e3902bf

alexeykudinkin force-pushed the ak/spkds-ref-1 branch from 37ce465 to e3902bf Compare March 4, 2022 21:34

Alexey Kudinkin added 7 commits March 4, 2022 13:37

Tidying up

b752778

Removing logs from MOR / Incremental test

2965980

Removed SparkHadoopUtil

880f4c9

Fixed HoodieWriteConfig to be able to disable compression in Parquet

be33a18

Disable compression in MOR / Incremental test to make test stable

c048190

Disable MOR / Incremental test

ed632b8

Disable full-table-read tests as unstable in Spark 2.x

97b6d31

nsivabalan reviewed Mar 6, 2022

View reviewed changes

Alexey Kudinkin added 2 commits March 7, 2022 12:12

Incorrect schema ref

1ecd626

Fixing compilation

448555f

nsivabalan approved these changes Mar 9, 2022

View reviewed changes

nsivabalan merged commit 034adda into apache:master Mar 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HUDI-3396] Make sure `BaseFileOnlyViewRelation` only reads projected columns #4818

[HUDI-3396] Make sure `BaseFileOnlyViewRelation` only reads projected columns #4818

alexeykudinkin commented Feb 15, 2022 •

edited

Loading

alexeykudinkin commented Feb 19, 2022

nsivabalan left a comment •

edited

Loading

nsivabalan Feb 26, 2022

alexeykudinkin Feb 26, 2022

nsivabalan Mar 6, 2022

alexeykudinkin commented Mar 5, 2022

nsivabalan Mar 6, 2022

nsivabalan left a comment

alexeykudinkin commented Mar 8, 2022

hudi-bot commented Mar 8, 2022

nsivabalan left a comment

[HUDI-3396] Make sure BaseFileOnlyViewRelation only reads projected columns #4818

[HUDI-3396] Make sure BaseFileOnlyViewRelation only reads projected columns #4818

Conversation

alexeykudinkin commented Feb 15, 2022 • edited Loading

Tips

What is the purpose of the pull request

Brief change log

Verify this pull request

Committer checklist

alexeykudinkin commented Feb 19, 2022

nsivabalan left a comment • edited Loading

Choose a reason for hiding this comment

nsivabalan Feb 26, 2022

Choose a reason for hiding this comment

alexeykudinkin Feb 26, 2022

Choose a reason for hiding this comment

nsivabalan Mar 6, 2022

Choose a reason for hiding this comment

alexeykudinkin commented Mar 5, 2022

nsivabalan Mar 6, 2022

Choose a reason for hiding this comment

nsivabalan left a comment

Choose a reason for hiding this comment

alexeykudinkin commented Mar 8, 2022

hudi-bot commented Mar 8, 2022

CI report:

nsivabalan left a comment

Choose a reason for hiding this comment

[HUDI-3396] Make sure `BaseFileOnlyViewRelation` only reads projected columns #4818

[HUDI-3396] Make sure `BaseFileOnlyViewRelation` only reads projected columns #4818

alexeykudinkin commented Feb 15, 2022 •

edited

Loading

nsivabalan left a comment •

edited

Loading