Restore portable schema changes, with simple LogicalType support by TheNeuralBit · Pull Request #9604 · apache/beam

TheNeuralBit · 2019-09-18T01:11:08Z

Un-reverts changes to the portable schema representation, with a couple of changes (22f3852):

Changes the LogicalType string args to a byte payload
Supports encoding arbitrary Java SDK LogicalTypes with the beam:fieldtype:javasdk URN. Long-term I would like to move types to a registry and discourage this URN (BEAM-7855), but for now it replicates the current implementation.

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).
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.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang	SDK	Apex	Dataflow	Gearpump	Samza	Spark
Go		---	---	---	---
Java
Python		---		---	---
XLang	---	---	---	---	---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website
Non-portable
Portable	---		---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

…Translation (apache#8853)"" This reverts commit dbcb14c.

…anup"" This reverts commit c9da964.

TheNeuralBit · 2019-09-18T01:11:22Z

Run Dataflow ValidatesRunner

TheNeuralBit · 2019-09-18T18:29:46Z

Run JavaPortabilityApi PreCommit

TheNeuralBit · 2019-09-18T22:18:54Z

R: @reuvenlax could you take a look at this? It restores my changes to the schema proto, but with a modification to represent all logical types with a "javasdk" urn and serialized java object in the payload (for now).

I think the precommit failure is unrelated, Hannah tells me that #9610 should resolve it.

I ran the Dataflow VR tests and also merged in #9446 locally to make sure the CloudObjectsTest works

robinyqiu · 2019-09-18T23:48:22Z


- private static RunnerApi.Schema.Field toProto(Field field, int fieldId, int position) {
- return RunnerApi.Schema.Field.newBuilder()
+ private static SchemaApi.Field toProto(Field field, int fieldId, int position) {


Rename to fieldToProto (to be consistent with fieldFromProto)?

robinyqiu · 2019-09-18T23:48:48Z

- RunnerApi.Schema.FieldType.Builder builder =
- RunnerApi.Schema.FieldType.newBuilder()
- .setTypeName(TYPE_NAME_MAPPING.get(fieldType.getTypeName()));
+ private static SchemaApi.FieldType toProto(FieldType fieldType) {


Similarly, fieldTypeToProto

robinyqiu · 2019-09-19T00:04:43Z

+
+ private static final String URN_BEAM_LOGICAL_DATETIME = "beam:fieldtype:datetime";
+ private static final String URN_BEAM_LOGICAL_DECIMAL = "beam:fieldtype:decimal";
+ private static final String URN_BEAM_LOGICAL_JAVASDK = "beam:fieldtype:javasdk";


Should we rename this URN to beam:schema:logical_type:javasdk:v1 (similar above)? It sounds clearer to me, and I saw other URNs have version numbers
beam/model/pipeline/src/main/proto/beam_runner_api.proto
Line 555 in 909bc5a
BYTES=0 [(beam_urn) = "beam:coder:bytes:v1"];

Hannah-Jiang · 2019-09-19T00:23:03Z

Run JavaPortabilityApi PreCommit

reuvenlax · 2019-09-19T16:49:37Z

Run Java PreCommit

reuvenlax · 2019-09-19T16:49:48Z

Run Python PreCommit

reuvenlax · 2019-09-19T16:50:02Z

Run Dataflow ValidatesRunner

reuvenlax · 2019-09-19T17:07:10Z

Run SQL PostCommit

TheNeuralBit · 2019-09-19T17:10:59Z

Thank you @Hannah-Jiang! looks like syncing past #9610 fixed the precommit issue :)

reuvenlax · 2019-09-19T17:11:10Z

Run Dataflow Runner Nexmark Tests

reuvenlax · 2019-09-19T17:11:19Z

LGTM. Will merge when all tests pass.

TheNeuralBit · 2019-09-19T17:13:02Z

I'll write a patch for @robinyqiu's naming suggestions as well

reuvenlax · 2019-09-19T17:35:47Z

Run Dataflow Runner Nexmark Tests

reuvenlax · 2019-09-19T17:35:56Z

Run Dataflow ValidatesRunner

reuvenlax · 2019-09-19T17:36:03Z

Run SQL PostCommit

reuvenlax · 2019-09-19T20:13:44Z

Run Python PreCommit

TheNeuralBit · 2019-09-19T21:04:28Z

BEAM-8286 seems to be the cause of the Python PreCommit failure

TheNeuralBit · 2019-09-19T21:05:05Z

Whoops fat fingered the "close and comment"

TheNeuralBit · 2019-09-19T23:35:04Z

Merged past #9620 which should fix the python precommit

reuvenlax · 2019-09-20T19:08:40Z

run Dataflow ValidatesRunner

reuvenlax · 2019-09-20T19:08:48Z

run Flink ValidatesRunner

reuvenlax · 2019-09-20T19:08:54Z

run SQL PostCommit

reuvenlax · 2019-09-20T19:09:04Z

run Portable Dataflow ValidatesRunner

TheNeuralBit added 3 commits September 17, 2019 17:25

Revert "Revert "Update portable schema representation and java Schema…
f629584
…Translation (apache#8853)"" This reverts commit dbcb14c.

Revert "Revert "Merge pull requestapache#8943: Schema conversion cle…
8f498db
…anup"" This reverts commit c9da964.

Support all logical types by encoding serialized LogicalType instance
22f3852

robinyqiu reviewed Sep 19, 2019
View reviewed changes

Merge remote-tracking branch 'origin/master' into revert-revert-schema
3e3e2d5

adjust logical type urns, make toProto function names more descriptive
7f53ee5

TheNeuralBit closed this Sep 19, 2019

TheNeuralBit reopened this Sep 19, 2019

Merge remote-tracking branch 'origin/master' into revert-revert-schema
bf27954

reuvenlax merged commit 7aa42a7 into apache:masterSep 21, 2019

Uh oh!

Conversation

TheNeuralBit commented Sep 18, 2019

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

Uh oh!

TheNeuralBit commented Sep 18, 2019

Uh oh!

TheNeuralBit commented Sep 18, 2019

Uh oh!

TheNeuralBit commented Sep 18, 2019

Uh oh!

robinyqiuSep 18, 2019

Choose a reason for hiding this comment

Uh oh!

robinyqiuSep 18, 2019

Choose a reason for hiding this comment

Uh oh!

robinyqiuSep 19, 2019

Choose a reason for hiding this comment

Uh oh!

Hannah-Jiang commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

TheNeuralBit commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

TheNeuralBit commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 19, 2019

Uh oh!

TheNeuralBit commented Sep 19, 2019

Uh oh!

TheNeuralBit commented Sep 19, 2019

Uh oh!

TheNeuralBit commented Sep 19, 2019

Uh oh!

reuvenlax commented Sep 20, 2019

Uh oh!

reuvenlax commented Sep 20, 2019

Uh oh!

reuvenlax commented Sep 20, 2019

Uh oh!

reuvenlax commented Sep 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants