Skip to content

Commit

Permalink
fix: addressing reviews v3
Browse files Browse the repository at this point in the history
  • Loading branch information
vibhatha committed Oct 25, 2023
1 parent 4c6b42c commit aeaa0f2
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down
24 changes: 3 additions & 21 deletions core/src/test/java/io/substrait/type/proto/JoinRoundtripTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -63,15 +49,11 @@ void verifyRoundTrip(Rel rel) {

@Test
void hashJoin() {
List<Integer> leftEmptyKeys = Arrays.asList(0, 1);
List<Integer> rightEmptyKeys = Arrays.asList(2, 0);
List<Integer> leftKeys = Arrays.asList(0, 1);
List<Integer> 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);
}
Expand Down

0 comments on commit aeaa0f2

Please sign in to comment.