From aeaa0f2f544db61b014d82aefee613cffc5a42ba Mon Sep 17 00:00:00 2001 From: Vibhatha Lakmal Abeykoon Date: Wed, 25 Oct 2023 06:26:49 +0530 Subject: [PATCH] fix: addressing reviews v3 --- .../substrait/relation/ProtoRelConverter.java | 4 +--- .../type/proto/JoinRoundtripTest.java | 24 +++---------------- 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/io/substrait/relation/ProtoRelConverter.java b/core/src/main/java/io/substrait/relation/ProtoRelConverter.java index 8d5641f3..9ae2cb95 100644 --- a/core/src/main/java/io/substrait/relation/ProtoRelConverter.java +++ b/core/src/main/java/io/substrait/relation/ProtoRelConverter.java @@ -39,7 +39,6 @@ import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.IntStream; -import java.util.stream.Stream; /** Converts from {@link io.substrait.proto.Rel} to {@link io.substrait.relation.Rel} */ public class ProtoRelConverter { @@ -501,8 +500,7 @@ private Rel newHashJoin(HashJoinRel rel) { Rel right = from(rel.getRight()); var leftKeys = rel.getLeftKeysList(); var rightKeys = rel.getRightKeysList(); - var rightOffSetKeys = - Stream.concat(leftKeys.stream(), rightKeys.stream()).collect(Collectors.toList()); + Type.Struct leftStruct = left.getRecordType(); Type.Struct rightStruct = right.getRecordType(); Type.Struct unionedStruct = Type.Struct.builder().from(leftStruct).from(rightStruct).build(); diff --git a/core/src/test/java/io/substrait/type/proto/JoinRoundtripTest.java b/core/src/test/java/io/substrait/type/proto/JoinRoundtripTest.java index c12d1cb3..2204178f 100644 --- a/core/src/test/java/io/substrait/type/proto/JoinRoundtripTest.java +++ b/core/src/test/java/io/substrait/type/proto/JoinRoundtripTest.java @@ -4,14 +4,12 @@ import io.substrait.TestBase; import io.substrait.dsl.SubstraitBuilder; -import io.substrait.extension.AdvancedExtension; import io.substrait.extension.ExtensionCollector; import io.substrait.extension.SimpleExtension; import io.substrait.relation.ProtoRelConverter; import io.substrait.relation.Rel; import io.substrait.relation.RelProtoConverter; import io.substrait.relation.physical.HashJoin; -import io.substrait.relation.utils.StringHolder; import io.substrait.relation.utils.StringHolderHandlingProtoRelConverter; import io.substrait.type.TypeCreator; import java.util.Arrays; @@ -43,18 +41,6 @@ public class JoinRoundtripTest extends TestBase { Arrays.asList("d", "e", "f"), Arrays.asList(R.FP64, R.STRING, R.I64)); - final AdvancedExtension commonExtension = - AdvancedExtension.builder() - .enhancement(new StringHolder("COMMON ENHANCEMENT")) - .optimization(new StringHolder("COMMON OPTIMIZATION")) - .build(); - - final AdvancedExtension relExtension = - AdvancedExtension.builder() - .enhancement(new StringHolder("REL ENHANCEMENT")) - .optimization(new StringHolder("REL OPTIMIZATION")) - .build(); - void verifyRoundTrip(Rel rel) { io.substrait.proto.Rel protoRel = relProtoConverter.toProto(rel); Rel relReturned = protoRelConverter.from(protoRel); @@ -63,15 +49,11 @@ void verifyRoundTrip(Rel rel) { @Test void hashJoin() { - List leftEmptyKeys = Arrays.asList(0, 1); - List rightEmptyKeys = Arrays.asList(2, 0); + List leftKeys = Arrays.asList(0, 1); + List rightKeys = Arrays.asList(2, 0); Rel relWithoutKeys = HashJoin.builder() - .from( - b.hashJoin( - leftEmptyKeys, rightEmptyKeys, HashJoin.JoinType.INNER, leftTable, rightTable)) - .commonExtension(commonExtension) - .extension(relExtension) + .from(b.hashJoin(leftKeys, rightKeys, HashJoin.JoinType.INNER, leftTable, rightTable)) .build(); verifyRoundTrip(relWithoutKeys); }