You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by GitBox <gi...@apache.org> on 2020/02/27 18:32:55 UTC
[GitHub] [beam] reuvenlax opened a new pull request #10990: disable coder
inference for rows
reuvenlax opened a new pull request #10990: disable coder inference for rows
URL: https://github.com/apache/beam/pull/10990
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] reuvenlax commented on issue #10990: [BEAM-9569] disable
coder inference for rows
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #10990: [BEAM-9569] disable coder inference for rows
URL: https://github.com/apache/beam/pull/10990#issuecomment-602424784
R: @dpmills
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] kennknowles commented on a change in pull request #10990:
disable coder inference for rows
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #10990: disable coder inference for rows
URL: https://github.com/apache/beam/pull/10990#discussion_r385304944
##########
File path: sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/transform/BeamSetOperatorsTransforms.java
##########
@@ -38,61 +35,62 @@
}
/** Filter function used for Set operators. */
- public static class SetOperatorFilteringDoFn extends DoFn<KV<Row, CoGbkResult>, Row> {
- private TupleTag<Row> leftTag;
- private TupleTag<Row> rightTag;
- private BeamSetOperatorRelBase.OpType opType;
+ public static class SetOperatorFilteringDoFn extends DoFn<Row, Row> {
+ private final String leftTag;
+ private final String rightTag;
+ private final String numRowsField;
+ private final BeamSetOperatorRelBase.OpType opType;
// ALL?
- private boolean all;
+ private final boolean all;
public SetOperatorFilteringDoFn(
- TupleTag<Row> leftTag,
- TupleTag<Row> rightTag,
+ String leftTag,
+ String rightTag,
+ String numRowsField,
BeamSetOperatorRelBase.OpType opType,
boolean all) {
this.leftTag = leftTag;
this.rightTag = rightTag;
+ this.numRowsField = numRowsField;
this.opType = opType;
this.all = all;
}
@ProcessElement
- public void processElement(ProcessContext ctx) {
- CoGbkResult coGbkResult = ctx.element().getValue();
- Iterable<Row> leftRows = coGbkResult.getAll(leftTag);
- Iterable<Row> rightRows = coGbkResult.getAll(rightTag);
+ public void processElement(@Element Row element, OutputReceiver<Row> o) {
+ Row key = element.getRow("key");
+ long numLeftRows = 0;
+ long numRightRows = 0;
+ if (!Iterables.isEmpty(element.<Row>getIterable(leftTag))) {
+ numLeftRows =
+ Iterables.getOnlyElement(element.<Row>getIterable(leftTag)).getInt64(numRowsField);
+ }
+ if (!Iterables.isEmpty(element.<Row>getIterable(rightTag))) {
+ numRightRows =
+ Iterables.getOnlyElement(element.<Row>getIterable(rightTag)).getInt64(numRowsField);
+ }
+
switch (opType) {
case UNION:
if (all) {
- // output both left & right
- Iterator<Row> iter = leftRows.iterator();
- while (iter.hasNext()) {
- ctx.output(iter.next());
- }
- iter = rightRows.iterator();
- while (iter.hasNext()) {
- ctx.output(iter.next());
+ for (int i = 0; i < numLeftRows + numRightRows; i++) {
+ o.output(key);
Review comment:
This optimization seems to have nothing to do with disabling coder inference. Please split into separate commit so git history shows it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] kennknowles commented on a change in pull request #10990:
disable coder inference for rows
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #10990: disable coder inference for rows
URL: https://github.com/apache/beam/pull/10990#discussion_r385303509
##########
File path: sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamSetOperatorRelBase.java
##########
@@ -65,6 +66,16 @@ public BeamSetOperatorRelBase(BeamRelNode beamRelNode, OpType opType, boolean al
inputs);
PCollection<Row> leftRows = inputs.get(0);
PCollection<Row> rightRows = inputs.get(1);
+ Schema leftSchema = leftRows.getSchema();
+ Schema rightSchema = rightRows.getSchema();
+ if (!leftSchema.typesEqual(rightSchema)) {
+ throw new IllegalArgumentException(
+ "Can't intersect two tables with different schemas."
Review comment:
This is the base class, so shouldn't be calling it "intersect". Incidentally I don't think having a base class adds much value here, so inlining or inverting would be a-ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] reuvenlax commented on issue #10990: [BEAM-9569] disable
coder inference for rows
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #10990: [BEAM-9569] disable coder inference for rows
URL: https://github.com/apache/beam/pull/10990#issuecomment-602766174
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] reuvenlax commented on issue #10990: disable coder inference
for rows
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #10990: disable coder inference for rows
URL: https://github.com/apache/beam/pull/10990#issuecomment-598493520
@kennknowles the Select changes have now been rebased away
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] dpmills commented on issue #10990: [BEAM-9569] disable coder
inference for rows
Posted by GitBox <gi...@apache.org>.
dpmills commented on issue #10990: [BEAM-9569] disable coder inference for rows
URL: https://github.com/apache/beam/pull/10990#issuecomment-602781033
LGTM
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] reuvenlax merged pull request #10990: [BEAM-9569] disable
coder inference for rows
Posted by GitBox <gi...@apache.org>.
reuvenlax merged pull request #10990: [BEAM-9569] disable coder inference for rows
URL: https://github.com/apache/beam/pull/10990
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] reuvenlax commented on issue #10990: disable coder inference
for rows
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on issue #10990: disable coder inference for rows
URL: https://github.com/apache/beam/pull/10990#issuecomment-598310772
@kennknowles I've removed the premature optimization. I decided to leave the base class for now - would prefer to leave that as a refactoring cleanup for another PR.
Note: support for nullable fields in this PR (as this PR exposed bugs in the Schema transforms) is also pulled out into pr/11046. I plan on merging 11046 first and rebasing this one on top, so those changes will not be included in this merge.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services