Adding support for Beam Schema Rows with BQ DIRECT_READ #22926

pabloem · 2022-08-26T23:08:33Z

Please add a meaningful description for your change here

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

pabloem · 2022-08-27T00:41:37Z

Run Java PostCommit

pabloem · 2022-08-27T00:48:40Z

r: @svetakvsundhar
r: @TheNeuralBit

This adds only schema support for table reads, not for query reads but it may not be difficult to add it for query reads.

svetakvsundhar

Thanks, this looks helpful, will wait for @TheNeuralBit 's feedback :)

Because you are defining fromBeamRow, is this also looking to be supported in WriteToBQ?

svetakvsundhar · 2022-08-27T21:26:45Z

...a/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.java

+
+          beamSchema = Schema.builder().addFields(flds).build();
+          SerializableFunction<T, Row> toBeamRow = getToBeamRowFn().apply(beamSchema);
+          SerializableFunction<Row, T> fromBeamRow = getFromBeamRowFn().apply(beamSchema);


is the purpose of this here when we wan't to convert something from a BeamRow type into another type? (e.g. the WriteToBQ case)

yes right! It relies on BQUtils to generate a converter function.

We already have this logic in BigQueryIO.expand (only triggered in the non direct read path). I wonder if we can refactor and re-use that path for both paths? Similar to what Svetak did in Python.

github-actions · 2022-08-28T17:36:00Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @robertwb for label java.
R: @johnjcasey for label io.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

pabloem · 2022-08-29T18:40:25Z

@TheNeuralBit PTAL : )

pabloem · 2022-09-01T20:25:45Z

PTAL? : D I would like to have this in snapshots to build and test the Syndeo template that I'm working on

TheNeuralBit · 2022-09-01T21:09:42Z

PTAL? : D I would like to have this in snapshots to build and test the Syndeo template that I'm working on

Ah shoot sorry. I will take a look today. One high-level question - I wonder if there's some duplication between this and the BQ TableProvider? It could be nice to have one defer to the other if possible.

TheNeuralBit · 2022-09-01T21:51:31Z

Run Java PostCommit

TheNeuralBit

LGTM overall, but I have a few suggestions. Thank you!

TheNeuralBit · 2022-09-01T21:57:59Z

...o/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/testing/FakeJobService.java

@@ -320,7 +320,9 @@ public Job getJob(JobReference jobRef) {
                              "Job %s failed: %s", job.job.getConfiguration(), e.toString())));
          List<ResourceId> sourceFiles =
              filesForLoadJobs.get(jobRef.getProjectId(), jobRef.getJobId());
-          FileSystems.delete(sourceFiles);


You might consider enabling the nullness checker in this file.

TheNeuralBit · 2022-09-01T21:58:19Z

...ud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIOStorageReadTest.java

+                Row.withSchema(beamSchema).addValue("C").build(),
+                Row.withSchema(beamSchema).addValue("D").build()));
+
+    p.run();


Can you also test with an integration test?

TheNeuralBit · 2022-09-01T21:59:09Z

...ud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIOStorageReadTest.java

+                    .from("foo.com:project:dataset.table")
+                    .withMethod(Method.DIRECT_READ)
+                    .withSelectedFields(Lists.newArrayList("name"))
+                    .withFormat(DataFormat.AVRO)


Will this work with Arrow format as well?

yes, it does (I've tested it on the syndeo template... and I've added an integration test)

TheNeuralBit · 2022-09-01T22:05:57Z

...a/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.java

+
+          beamSchema = Schema.builder().addFields(flds).build();
+          SerializableFunction<T, Row> toBeamRow = getToBeamRowFn().apply(beamSchema);
+          SerializableFunction<Row, T> fromBeamRow = getFromBeamRowFn().apply(beamSchema);


We already have this logic in BigQueryIO.expand (only triggered in the non direct read path). I wonder if we can refactor and re-use that path for both paths? Similar to what Svetak did in Python.

pabloem · 2022-09-02T05:13:26Z

Run Java PostCommit

pabloem · 2022-09-02T16:11:34Z

Run Java PreCommit

* Adding support for Beam Schema Rows with BQ DIRECT_READ * Fixing for trimmed-out fields * refactor * Fix NPE in FakeJobService * Addressing comments

…2926 and apache/beam/apache#22942 in snapshots

Adding support for Beam Schema Rows with BQ DIRECT_READ

c2c5421

github-actions bot added gcp io java labels Aug 26, 2022

Fixing for trimmed-out fields

aea2017

svetakvsundhar reviewed Aug 27, 2022

View reviewed changes

pabloem changed the title ~~[WIP] Adding support for Beam Schema Rows with BQ DIRECT_READ~~ Adding support for Beam Schema Rows with BQ DIRECT_READ Aug 28, 2022

github-actions bot added the Next Action: Reviewers label Aug 28, 2022

refactor

42ce95f

pabloem force-pushed the beam-schema-bq-direct-read branch from 43ed68b to 42ce95f Compare August 29, 2022 17:21

Fix NPE in FakeJobService

f22c62a

TheNeuralBit self-requested a review August 29, 2022 23:53

TheNeuralBit approved these changes Sep 1, 2022

View reviewed changes

Addressing comments

78ac016

pabloem merged commit 948af30 into apache:master Sep 2, 2022

pabloem deleted the beam-schema-bq-direct-read branch September 2, 2022 16:51

Abacn pushed a commit to Abacn/beam that referenced this pull request Jan 31, 2023

Enabling local and integration tests. Depends on apache/beam/apache#2…

5b2b0fa

…2926 and apache/beam/apache#22942 in snapshots

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for Beam Schema Rows with BQ DIRECT_READ #22926

Adding support for Beam Schema Rows with BQ DIRECT_READ #22926

pabloem commented Aug 26, 2022

pabloem commented Aug 27, 2022

pabloem commented Aug 27, 2022

svetakvsundhar left a comment

svetakvsundhar Aug 27, 2022

pabloem Aug 28, 2022

TheNeuralBit Sep 1, 2022

pabloem Sep 2, 2022

github-actions bot commented Aug 28, 2022

pabloem commented Aug 29, 2022

pabloem commented Sep 1, 2022

TheNeuralBit commented Sep 1, 2022

TheNeuralBit commented Sep 1, 2022

TheNeuralBit left a comment

TheNeuralBit Sep 1, 2022

TheNeuralBit Sep 1, 2022

pabloem Sep 2, 2022

TheNeuralBit Sep 1, 2022

pabloem Sep 2, 2022

TheNeuralBit Sep 1, 2022

pabloem commented Sep 2, 2022

pabloem commented Sep 2, 2022

Adding support for Beam Schema Rows with BQ DIRECT_READ #22926

Adding support for Beam Schema Rows with BQ DIRECT_READ #22926

Conversation

pabloem commented Aug 26, 2022

GitHub Actions Tests Status (on master branch)

pabloem commented Aug 27, 2022

pabloem commented Aug 27, 2022

svetakvsundhar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 28, 2022

pabloem commented Aug 29, 2022

pabloem commented Sep 1, 2022

TheNeuralBit commented Sep 1, 2022

TheNeuralBit commented Sep 1, 2022

TheNeuralBit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloem commented Sep 2, 2022

pabloem commented Sep 2, 2022