Operators In / Not In are not implemented for Residuals

[arina-ielchiieva]

Example of the unit tests in org.apache.iceberg.transforms.TestResiduals which should pass:

  @Test
  public void testIn() {
    Schema schema = new Schema(
      Types.NestedField.optional(50, "dateint", Types.IntegerType.get()),
      Types.NestedField.optional(51, "hour", Types.IntegerType.get())
    );

    PartitionSpec spec = PartitionSpec.builderFor(schema)
      .identity("dateint")
      .build();

    ResidualEvaluator resEval = ResidualEvaluator.of(spec, 
      in("dateint", 20170815, 20170816, 20170817), true);

    Expression residual = resEval.residualFor(Row.of(20170815));
    Assert.assertEquals("Residual should be alwaysTrue", alwaysTrue(), residual);
  }

  @Test
  public void testNotIn() {
    Schema schema = new Schema(
      Types.NestedField.optional(50, "dateint", Types.IntegerType.get()),
      Types.NestedField.optional(51, "hour", Types.IntegerType.get())
    );

    PartitionSpec spec = PartitionSpec.builderFor(schema)
      .identity("dateint")
      .build();

    ResidualEvaluator resEval = ResidualEvaluator.of(spec, 
      notIn("dateint", 20170815, 20170816, 20170817),true);

    Expression residual = resEval.residualFor(Row.of(20170815));
    Assert.assertEquals("Residual should be alwaysFalse", alwaysFalse(), residual);
  }

Now both unit tests fail. Example of the exception:

java.lang.UnsupportedOperationException: In operation is not supported by the visitor

	at org.apache.iceberg.expressions.ExpressionVisitors$BoundExpressionVisitor.in(ExpressionVisitors.java:102)
	at org.apache.iceberg.expressions.ExpressionVisitors$BoundExpressionVisitor.predicate(ExpressionVisitors.java:152)
	at org.apache.iceberg.expressions.ResidualEvaluator$ResidualVisitor.predicate(ResidualEvaluator.java:228)
	at org.apache.iceberg.expressions.ResidualEvaluator$ResidualVisitor.predicate(ResidualEvaluator.java:270)
	at org.apache.iceberg.expressions.ResidualEvaluator$ResidualVisitor.predicate(ResidualEvaluator.java:119)
	at org.apache.iceberg.expressions.ExpressionVisitors.visit(ExpressionVisitors.java:283)
	at org.apache.iceberg.expressions.ResidualEvaluator$ResidualVisitor.eval(ResidualEvaluator.java:124)
	at org.apache.iceberg.expressions.ResidualEvaluator$ResidualVisitor.access$100(ResidualEvaluator.java:119)
	at org.apache.iceberg.expressions.ResidualEvaluator.residualFor(ResidualEvaluator.java:116)
	at org.apache.iceberg.transforms.TestResiduals.testIn(TestResiduals.java:176)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:230)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:58)

I think #600 did not add these operators to all visitors, in particular ResidualVisitor was omitted.

[arina-ielchiieva]

FYI @jun-he

[jun-he]

Thanks @arina-ielchiieva.
I will add them to ResidualVisitor and double check other visitors.

[jun-he]

@arina-ielchiieva I have submitted a PR to address this issue. Thanks.

[arina-ielchiieva]

@jun-he thanks!

[jun-he]

Can we close it as the change is merged? Thanks.

[aokolnychyi]

@jun-he, thanks for fixing this!