You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/11/03 22:28:21 UTC
[GitHub] [beam] apilloud opened a new pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
apilloud opened a new pull request #15886:
URL: https://github.com/apache/beam/pull/15886
Allow FieldAccess information to be extracted from a schema enabled DoFn.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
`ValidatesRunner` compliance status (on master branch)
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/badge/icon?subject=V1+Streaming">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon?subject=V1+Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/badge/icon?subject=V2+Streaming">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon?subject=Java+8">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon?subject=Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon?subject=Portable+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/badge/icon?subject=Portable">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon?subject=Structured+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon?subject=ValCont">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
</tbody>
</table>
Examples testing status on various runners
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/badge/icon?subject=V1+Java11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
Post-Commit SDK/Transform Integration Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>Go</th>
<th>Java</th>
<th>Python</th>
</tr>
</thead>
<tbody>
<tr>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon?subject=3.6">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon?subject=3.7">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon?subject=3.8">
</a>
</td>
</tr>
</tbody>
</table>
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>---</th>
<th>Java</th>
<th>Python</th>
<th>Go</th>
<th>Website</th>
<th>Whitespace</th>
<th>Typescript</th>
</tr>
</thead>
<tbody>
<tr>
<td>Non-portable</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon?subject=Tests">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon?subject=Lint">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon?subject=Docker">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/badge/icon?subject=Docs">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Portable</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r745138773
##########
File path: sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRelTest.java
##########
@@ -213,17 +212,14 @@ public void testSingleFieldAccess() throws IllegalAccessException {
ParDo.MultiOutput<Row, Row> pardo =
(ParDo.MultiOutput<Row, Row>) nodeGetter.producer.getTransform();
- DoFnSignature sig = DoFnSignatures.getSignature(pardo.getFn().getClass());
+ PCollection<Row> input =
+ (PCollection<Row>) nodeGetter.producer.getInputs().values().iterator().next();
Review comment:
Done.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on pull request #15886:
URL: https://github.com/apache/beam/pull/15886#issuecomment-960246960
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #15886:
URL: https://github.com/apache/beam/pull/15886#issuecomment-960287547
Is this so that we can lift projections?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on pull request #15886:
URL: https://github.com/apache/beam/pull/15886#issuecomment-963661831
Run Java PreCommit
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r745268178
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -141,7 +150,27 @@ DoFnSchemaInformation withUnboxPrimitiveParameter(
elementT))
.build();
- return toBuilder().setElementConverters(converters).build();
+ return toBuilder()
+ .setElementConverters(converters)
+ .setFieldAccessDescriptor(getFieldAccessDescriptor())
Review comment:
Done.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on pull request #15886:
URL: https://github.com/apache/beam/pull/15886#issuecomment-960246960
R: @ibzib
cc: @reuvenlax
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on pull request #15886:
URL: https://github.com/apache/beam/pull/15886#issuecomment-963647845
Run Java_Examples_Dataflow PreCommit
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r742418785
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/ParDo.java
##########
@@ -683,6 +686,18 @@ public static DoFnSchemaInformation getDoFnSchemaInformation(
(SchemaCoder<?>) input.getCoder(), accessDescriptor, selectedSchema, elementT);
}
}
+ if (input.hasSchema()) {
Review comment:
unnecessary - we wouldn't hit this codepath if !input.hasSchema
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
+
+ return toBuilder()
+ .setElementConverters(converters)
+ .setFieldAccessDescriptors(descriptors)
+ .build();
Review comment:
This is not always called (withUnboxFromPrimitiveParameter as called as well). I think better to make this a separate builder called from ParDo.java
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
Review comment:
Instead of storing a List, just store one FieldAccessDescriptor and use FieldAccessDescriptor.union to merge them.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #15886:
URL: https://github.com/apache/beam/pull/15886#issuecomment-960287547
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on pull request #15886:
URL: https://github.com/apache/beam/pull/15886#issuecomment-960246960
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r743093232
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
+
+ return toBuilder()
+ .setElementConverters(converters)
+ .setFieldAccessDescriptors(descriptors)
+ .build();
Review comment:
I refactored this so setElementConverters and setFieldAccessDescriptors are separate methods.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud merged pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud merged pull request #15886:
URL: https://github.com/apache/beam/pull/15886
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r743092758
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/ParDo.java
##########
@@ -683,6 +686,18 @@ public static DoFnSchemaInformation getDoFnSchemaInformation(
(SchemaCoder<?>) input.getCoder(), accessDescriptor, selectedSchema, elementT);
}
}
+ if (input.hasSchema()) {
Review comment:
Removed.
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
+
+ return toBuilder()
+ .setElementConverters(converters)
+ .setFieldAccessDescriptors(descriptors)
+ .build();
Review comment:
I refactored this so setElementConverters and setFieldAccessDescriptors are separate methods.
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
Review comment:
Thanks for pointing that out! I was looking for that method.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ibzib commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r744033060
##########
File path: sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRelTest.java
##########
@@ -213,17 +212,14 @@ public void testSingleFieldAccess() throws IllegalAccessException {
ParDo.MultiOutput<Row, Row> pardo =
(ParDo.MultiOutput<Row, Row>) nodeGetter.producer.getTransform();
- DoFnSignature sig = DoFnSignatures.getSignature(pardo.getFn().getClass());
+ PCollection<Row> input =
+ (PCollection<Row>) nodeGetter.producer.getInputs().values().iterator().next();
Review comment:
It may be better to use `Iterables.getOnlyElement` instead. (likewise for other tests)
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r742418785
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/ParDo.java
##########
@@ -683,6 +686,18 @@ public static DoFnSchemaInformation getDoFnSchemaInformation(
(SchemaCoder<?>) input.getCoder(), accessDescriptor, selectedSchema, elementT);
}
}
+ if (input.hasSchema()) {
Review comment:
unnecessary - we wouldn't hit this codepath if !input.hasSchema
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
+
+ return toBuilder()
+ .setElementConverters(converters)
+ .setFieldAccessDescriptors(descriptors)
+ .build();
Review comment:
This is not always called (withUnboxFromPrimitiveParameter as called as well). I think better to make this a separate builder called from ParDo.java
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
Review comment:
Instead of storing a List, just store one FieldAccessDescriptor and use FieldAccessDescriptor.union to merge them.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r743092758
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/ParDo.java
##########
@@ -683,6 +686,18 @@ public static DoFnSchemaInformation getDoFnSchemaInformation(
(SchemaCoder<?>) input.getCoder(), accessDescriptor, selectedSchema, elementT);
}
}
+ if (input.hasSchema()) {
Review comment:
Removed.
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
+
+ return toBuilder()
+ .setElementConverters(converters)
+ .setFieldAccessDescriptors(descriptors)
+ .build();
Review comment:
I refactored this so setElementConverters and setFieldAccessDescriptors are separate methods.
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
Review comment:
Thanks for pointing that out! I was looking for that method.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r742418785
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/ParDo.java
##########
@@ -683,6 +686,18 @@ public static DoFnSchemaInformation getDoFnSchemaInformation(
(SchemaCoder<?>) input.getCoder(), accessDescriptor, selectedSchema, elementT);
}
}
+ if (input.hasSchema()) {
Review comment:
unnecessary - we wouldn't hit this codepath if !input.hasSchema
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
+
+ return toBuilder()
+ .setElementConverters(converters)
+ .setFieldAccessDescriptors(descriptors)
+ .build();
Review comment:
This is not always called (withUnboxFromPrimitiveParameter as called as well). I think better to make this a separate builder called from ParDo.java
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
Review comment:
Instead of storing a List, just store one FieldAccessDescriptor and use FieldAccessDescriptor.union to merge them.
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/ParDo.java
##########
@@ -683,6 +686,18 @@ public static DoFnSchemaInformation getDoFnSchemaInformation(
(SchemaCoder<?>) input.getCoder(), accessDescriptor, selectedSchema, elementT);
}
}
+ if (input.hasSchema()) {
Review comment:
unnecessary - we wouldn't hit this codepath if !input.hasSchema
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
+
+ return toBuilder()
+ .setElementConverters(converters)
+ .setFieldAccessDescriptors(descriptors)
+ .build();
Review comment:
This is not always called (withUnboxFromPrimitiveParameter as called as well). I think better to make this a separate builder called from ParDo.java
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
Review comment:
Instead of storing a List, just store one FieldAccessDescriptor and use FieldAccessDescriptor.union to merge them.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #15886:
URL: https://github.com/apache/beam/pull/15886#issuecomment-960287547
Is this so that we can lift projections?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on pull request #15886:
URL: https://github.com/apache/beam/pull/15886#issuecomment-960291409
Yes, this is working towards lifting projections.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r743093527
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -101,7 +107,16 @@ DoFnSchemaInformation withSelectFromSchemaParameter(
unbox))
.build();
- return toBuilder().setElementConverters(converters).build();
+ List<FieldAccessDescriptor> descriptors =
+ ImmutableList.<FieldAccessDescriptor>builder()
+ .addAll(getFieldAccessDescriptors())
+ .add(selectDescriptor)
+ .build();
Review comment:
Thanks for pointing that out! I was looking for that method.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
apilloud commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r743092758
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/ParDo.java
##########
@@ -683,6 +686,18 @@ public static DoFnSchemaInformation getDoFnSchemaInformation(
(SchemaCoder<?>) input.getCoder(), accessDescriptor, selectedSchema, elementT);
}
}
+ if (input.hasSchema()) {
Review comment:
Removed.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on a change in pull request #15886: [BEAM-13056] Expose FieldAccess in DoFnSchemaInformation
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on a change in pull request #15886:
URL: https://github.com/apache/beam/pull/15886#discussion_r745240039
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFnSchemaInformation.java
##########
@@ -141,7 +150,27 @@ DoFnSchemaInformation withUnboxPrimitiveParameter(
elementT))
.build();
- return toBuilder().setElementConverters(converters).build();
+ return toBuilder()
+ .setElementConverters(converters)
+ .setFieldAccessDescriptor(getFieldAccessDescriptor())
Review comment:
This shouldn't be necessary - only need to specify changed fields
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org