You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2019/07/15 16:38:57 UTC

[calcite] 01/06: [CALCITE-3183] During field trimming, Filter is copied with wrong traitSet (Juhwan Kim)

This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit bb0bae9c047db0f4bebaa6cd5b149876476e6ebe
Author: Juhwan Kim <jk...@dremio.com>
AuthorDate: Fri Jul 12 10:19:10 2019 -0700

    [CALCITE-3183] During field trimming, Filter is copied with wrong traitSet (Juhwan Kim)
    
    New Filter created after trimming uses its old trait set,
    which could cause problem if there has been any update on collation.
    
    Code changes:
    - Add a FilterFactory method that includes correlating variables;
    - Use RelBuilder when creating a new Filter in trimmer;
    - Add test cases for the bug and new RelBuilder method.
    
    Close apache/calcite#1309
---
 .../calcite/adapter/enumerable/EnumerableRel.java  |  9 ++-
 .../org/apache/calcite/adapter/jdbc/JdbcRules.java |  9 ++-
 .../java/org/apache/calcite/plan/RelOptUtil.java   | 11 ++--
 .../org/apache/calcite/rel/core/RelFactories.java  | 30 ++++++++--
 .../rel/rules/SemiJoinFilterTransposeRule.java     |  3 +-
 .../apache/calcite/sql2rel/RelFieldTrimmer.java    |  6 +-
 .../apache/calcite/sql2rel/SqlToRelConverter.java  |  5 +-
 .../java/org/apache/calcite/tools/RelBuilder.java  | 28 +++++++++-
 .../org/apache/calcite/test/RelBuilderTest.java    | 49 +++++++++++++++++
 .../apache/calcite/test/SqlToRelConverterTest.java | 64 ++++++++++++++++++++++
 core/src/test/resources/sql/misc.iq                |  4 +-
 .../adapter/geode/rel/GeodeAllDataTypesTest.java   |  4 +-
 .../calcite/adapter/pig/PigRelFactories.java       | 12 +++-
 site/_docs/algebra.md                              |  4 +-
 14 files changed, 209 insertions(+), 29 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRel.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRel.java
index 18f2594..fd80af7 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRel.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRel.java
@@ -20,6 +20,8 @@ import org.apache.calcite.linq4j.tree.BlockStatement;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.RelFactories;
 
+import com.google.common.base.Preconditions;
+
 /**
  * A relational expression of one of the
  * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention} calling
@@ -27,7 +29,12 @@ import org.apache.calcite.rel.core.RelFactories;
  */
 public interface EnumerableRel
     extends RelNode {
-  RelFactories.FilterFactory FILTER_FACTORY = EnumerableFilter::create;
+  RelFactories.FilterFactory FILTER_FACTORY =
+      (input, condition, variablesSet) -> {
+        Preconditions.checkArgument(variablesSet.isEmpty(),
+            "EnumerableFilter does not allow variables");
+        return EnumerableFilter.create(input, condition);
+      };
 
   RelFactories.ProjectFactory PROJECT_FACTORY = EnumerableProject::create;
 
diff --git a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
index b46835f..7e3e0da 100644
--- a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
+++ b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
@@ -74,6 +74,7 @@ import org.apache.calcite.util.ImmutableBitSet;
 import org.apache.calcite.util.Util;
 import org.apache.calcite.util.trace.CalciteTrace;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 
 import org.slf4j.Logger;
@@ -105,8 +106,12 @@ public class JdbcRules {
       };
 
   static final RelFactories.FilterFactory FILTER_FACTORY =
-      (input, condition) -> new JdbcRules.JdbcFilter(input.getCluster(),
-          input.getTraitSet(), input, condition);
+      (input, condition, variablesSet) -> {
+        Preconditions.checkArgument(variablesSet.isEmpty(),
+            "JdbcFilter does not allow variables");
+        return new JdbcFilter(input.getCluster(),
+            input.getTraitSet(), input, condition);
+      };
 
   static final RelFactories.JoinFactory JOIN_FACTORY =
       (left, right, condition, variablesSet, joinType, semiJoinDone) -> {
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index b795a42..5e8500c 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -104,6 +104,7 @@ import org.apache.calcite.util.mapping.MappingType;
 import org.apache.calcite.util.mapping.Mappings;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Lists;
@@ -453,7 +454,7 @@ public abstract class RelOptUtil {
 
       final RelFactories.FilterFactory factory =
           RelFactories.DEFAULT_FILTER_FACTORY;
-      ret = factory.createFilter(ret, conditionExp);
+      ret = factory.createFilter(ret, conditionExp, ImmutableSet.of());
     }
 
     if (extraExpr != null) {
@@ -602,13 +603,13 @@ public abstract class RelOptUtil {
   public static RelNode createFilter(RelNode child, RexNode condition) {
     final RelFactories.FilterFactory factory =
         RelFactories.DEFAULT_FILTER_FACTORY;
-    return factory.createFilter(child, condition);
+    return factory.createFilter(child, condition, ImmutableSet.of());
   }
 
   @Deprecated // to be removed before 2.0
   public static RelNode createFilter(RelNode child, RexNode condition,
       RelFactories.FilterFactory filterFactory) {
-    return filterFactory.createFilter(child, condition);
+    return filterFactory.createFilter(child, condition, ImmutableSet.of());
   }
 
   /** Creates a filter, using the default filter factory,
@@ -631,7 +632,7 @@ public abstract class RelOptUtil {
     if (condition == null) {
       return child;
     } else {
-      return filterFactory.createFilter(child, condition);
+      return filterFactory.createFilter(child, condition, ImmutableSet.of());
     }
   }
 
@@ -681,7 +682,7 @@ public abstract class RelOptUtil {
 
     final RelFactories.FilterFactory factory =
         RelFactories.DEFAULT_FILTER_FACTORY;
-    return factory.createFilter(rel, condition);
+    return factory.createFilter(rel, condition, ImmutableSet.of());
   }
 
   /**
diff --git a/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java b/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
index 728bc20..ff8af92 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
@@ -297,12 +297,30 @@ public class RelFactories {
   }
 
   /**
-   * Can create a {@link LogicalFilter} of the appropriate type
+   * Can create a {@link Filter} of the appropriate type
    * for this rule's calling convention.
    */
   public interface FilterFactory {
-    /** Creates a filter. */
-    RelNode createFilter(RelNode input, RexNode condition);
+    /** Creates a filter.
+     *
+     * <p>Some implementations of {@code Filter} do not support correlation
+     * variables, and for these, this method will throw if {@code variablesSet}
+     * is not empty.
+     *
+     * @param input Input relational expression
+     * @param condition Filter condition; only rows for which this condition
+     *   evaluates to TRUE will be emitted
+     * @param variablesSet Correlating variables that are set when reading
+     *   a row from the input, and which may be referenced from inside the
+     *   condition
+     */
+    RelNode createFilter(RelNode input, RexNode condition,
+        Set<CorrelationId> variablesSet);
+
+    @Deprecated // to be removed before 2.0
+    default RelNode createFilter(RelNode input, RexNode condition) {
+      return createFilter(input, condition, ImmutableSet.of());
+    }
   }
 
   /**
@@ -310,8 +328,10 @@ public class RelFactories {
    * returns a vanilla {@link LogicalFilter}.
    */
   private static class FilterFactoryImpl implements FilterFactory {
-    public RelNode createFilter(RelNode input, RexNode condition) {
-      return LogicalFilter.create(input, condition);
+    public RelNode createFilter(RelNode input, RexNode condition,
+        Set<CorrelationId> variablesSet) {
+      return LogicalFilter.create(input, condition,
+          ImmutableSet.copyOf(variablesSet));
     }
   }
 
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/SemiJoinFilterTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/SemiJoinFilterTransposeRule.java
index 16dc13c..2f2f3ca 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/SemiJoinFilterTransposeRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/SemiJoinFilterTransposeRule.java
@@ -72,7 +72,8 @@ public class SemiJoinFilterTransposeRule extends RelOptRule {
     final RelFactories.FilterFactory factory =
         RelFactories.DEFAULT_FILTER_FACTORY;
     RelNode newFilter =
-        factory.createFilter(newSemiJoin, filter.getCondition());
+        factory.createFilter(newSemiJoin, filter.getCondition(),
+            ImmutableSet.of());
 
     call.transformTo(newFilter);
   }
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
index b92d5a1..2d01a96 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
@@ -482,9 +482,9 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
     RexNode newConditionExpr =
         conditionExpr.accept(shuttle);
 
-    // Use copy rather than relBuilder so that correlating variables get set.
-    relBuilder.push(
-        filter.copy(filter.getTraitSet(), newInput, newConditionExpr));
+    // Build new filter with trimmed input and condition.
+    relBuilder.push(newInput)
+        .filter(filter.getVariablesSet(), newConditionExpr);
 
     // The result has the same mapping as the input gave us. Sometimes we
     // return fields that the consumer didn't ask for, because the filter
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
index 7668ee8..fe490a0 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -989,7 +989,8 @@ public class SqlToRelConverter {
 
     final RelFactories.FilterFactory filterFactory =
         RelFactories.DEFAULT_FILTER_FACTORY;
-    final RelNode filter = filterFactory.createFilter(bb.root, convertedWhere2);
+    final RelNode filter =
+        filterFactory.createFilter(bb.root, convertedWhere2, ImmutableSet.of());
     final RelNode r;
     final CorrelationUse p = getCorrelationUse(bb, filter);
     if (p != null) {
@@ -2490,7 +2491,7 @@ public class SqlToRelConverter {
         // Replace outer RexInputRef with RexFieldAccess,
         // and push lateral join predicate into inner child
         final RexNode newCond = joinCond.accept(shuttle);
-        innerRel = factory.createFilter(p.r, newCond);
+        innerRel = factory.createFilter(p.r, newCond, ImmutableSet.of());
         requiredCols = ImmutableBitSet
             .fromBitSet(shuttle.varCols)
             .union(p.requiredColumns);
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index c8545ed..7058e50 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -1146,7 +1146,7 @@ public class RelBuilder {
    * and optimized in a similar way to the {@link #and} method.
    * If the result is TRUE no filter is created. */
   public RelBuilder filter(RexNode... predicates) {
-    return filter(ImmutableList.copyOf(predicates));
+    return filter(ImmutableSet.of(), ImmutableList.copyOf(predicates));
   }
 
   /** Creates a {@link Filter} of a list of
@@ -1156,6 +1156,29 @@ public class RelBuilder {
    * and optimized in a similar way to the {@link #and} method.
    * If the result is TRUE no filter is created. */
   public RelBuilder filter(Iterable<? extends RexNode> predicates) {
+    return filter(ImmutableSet.of(), predicates);
+  }
+
+  /** Creates a {@link Filter} of a list of correlation variables
+   * and an array of predicates.
+   *
+   * <p>The predicates are combined using AND,
+   * and optimized in a similar way to the {@link #and} method.
+   * If the result is TRUE no filter is created. */
+  public RelBuilder filter(Iterable<CorrelationId> variablesSet,
+      RexNode... predicates) {
+    return filter(variablesSet, ImmutableList.copyOf(predicates));
+  }
+
+  /**
+   * Creates a {@link Filter} of a list of correlation variables
+   * and a list of predicates.
+   *
+   * <p>The predicates are combined using AND,
+   * and optimized in a similar way to the {@link #and} method.
+   * If the result is TRUE no filter is created. */
+  public RelBuilder filter(Iterable<CorrelationId> variablesSet,
+      Iterable<? extends RexNode> predicates) {
     final RexNode simplifiedPredicates =
         simplifier.simplifyFilterPredicates(predicates);
     if (simplifiedPredicates == null) {
@@ -1164,7 +1187,8 @@ public class RelBuilder {
 
     if (!simplifiedPredicates.isAlwaysTrue()) {
       final Frame frame = stack.pop();
-      final RelNode filter = filterFactory.createFilter(frame.rel, simplifiedPredicates);
+      final RelNode filter = filterFactory.createFilter(frame.rel,
+          simplifiedPredicates, ImmutableSet.copyOf(variablesSet));
       stack.push(new Frame(filter, frame.fields));
     }
     return this;
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index a71ce05..29e42c2 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -25,6 +25,7 @@ import org.apache.calcite.rel.RelDistributions;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.AggregateCall;
 import org.apache.calcite.rel.core.Correlate;
+import org.apache.calcite.rel.core.CorrelationId;
 import org.apache.calcite.rel.core.Exchange;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.core.RelFactories;
@@ -2587,6 +2588,54 @@ public class RelBuilderTest {
     assertThat(root, hasTree(expected));
   }
 
+  /** Tests filter builder with correlation variables */
+  @Test public void testFilterWithCorrelationVariables() {
+    final RelBuilder builder = RelBuilder.create(config().build());
+    final Holder<RexCorrelVariable> v = Holder.of(null);
+    RelNode root = builder.scan("EMP")
+        .variable(v)
+        .scan("DEPT")
+        .filter(Collections.singletonList(v.get().id),
+            builder.call(SqlStdOperatorTable.OR,
+                builder.call(SqlStdOperatorTable.AND,
+                    builder.call(SqlStdOperatorTable.LESS_THAN,
+                        builder.field(v.get(), "DEPTNO"),
+                        builder.literal(30)),
+                    builder.call(SqlStdOperatorTable.GREATER_THAN,
+                        builder.field(v.get(), "DEPTNO"),
+                        builder.literal(20))),
+                builder.isNull(builder.field(2))))
+        .join(JoinRelType.LEFT,
+            builder.equals(builder.field(2, 0, "SAL"),
+                builder.literal(1000)),
+            ImmutableSet.of(v.get().id))
+        .build();
+
+    final String expected = ""
+        + "LogicalCorrelate(correlation=[$cor0], joinType=[left], requiredColumns=[{7}])\n"
+        + "  LogicalTableScan(table=[[scott, EMP]])\n"
+        + "  LogicalFilter(condition=[=($cor0.SAL, 1000)])\n"
+        + "    LogicalFilter(condition=[OR(AND(<($cor0.DEPTNO, 30), >($cor0.DEPTNO, 20)), "
+        + "IS NULL($2))], variablesSet=[[$cor0]])\n"
+        + "      LogicalTableScan(table=[[scott, DEPT]])\n";
+
+    assertThat(root, hasTree(expected));
+  }
+
+  @Test public void testFilterEmpty() {
+    final RelBuilder builder = RelBuilder.create(config().build());
+    final RelNode root =
+        builder.scan("EMP")
+            // We intend to call
+            //   filter(Iterable<CorrelationId>, RexNode...)
+            // with zero varargs, not
+            //   filter(Iterable<RexNode>)
+            // Let's hope they're distinct after type erasure.
+            .filter(ImmutableSet.<CorrelationId>of())
+            .build();
+    assertThat(root, hasTree("LogicalTableScan(table=[[scott, EMP]])\n"));
+  }
+
   @Test public void testRelBuilderToString() {
     final RelBuilder builder = RelBuilder.create(config().build());
     builder.scan("EMP");
diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index 3499657..a7d2ae5 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -20,12 +20,20 @@ import org.apache.calcite.config.CalciteConnectionConfigImpl;
 import org.apache.calcite.config.CalciteConnectionProperty;
 import org.apache.calcite.config.NullCollation;
 import org.apache.calcite.plan.Contexts;
+import org.apache.calcite.plan.RelOptPlanner;
 import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.plan.RelTrait;
+import org.apache.calcite.plan.RelTraitDef;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollationTraitDef;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.RelRoot;
+import org.apache.calcite.rel.RelShuttleImpl;
 import org.apache.calcite.rel.RelVisitor;
 import org.apache.calcite.rel.core.CorrelationId;
 import org.apache.calcite.rel.externalize.RelXmlWriter;
+import org.apache.calcite.rel.logical.LogicalFilter;
+import org.apache.calcite.rel.logical.LogicalSort;
 import org.apache.calcite.sql.SqlExplainLevel;
 import org.apache.calcite.sql.validate.SqlConformance;
 import org.apache.calcite.sql.validate.SqlConformanceEnum;
@@ -37,6 +45,7 @@ import org.apache.calcite.util.Litmus;
 import org.apache.calcite.util.TestUtil;
 import org.apache.calcite.util.Util;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 
 import org.junit.Ignore;
@@ -45,12 +54,16 @@ import org.junit.Test;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.ArrayDeque;
+import java.util.ArrayList;
 import java.util.Deque;
+import java.util.List;
 import java.util.Properties;
 import java.util.Set;
 
 import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Unit test for {@link org.apache.calcite.sql2rel.SqlToRelConverter}.
@@ -1926,6 +1939,57 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
     sql(sql).trim(true).ok();
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-3183">[CALCITE-3183]
+   * Trimming method for Filter rel uses wrong traitSet </a> */
+  @Test public void testFilterAndSortWithTrim() {
+    // Create a customized test with RelCollation trait in the test cluster.
+    Tester tester = new TesterImpl(getDiffRepos(),
+        false, true,
+        true, false,
+        null, null) {
+      @Override public RelOptPlanner createPlanner() {
+        return new MockRelOptPlanner(Contexts.empty()) {
+          @Override public List<RelTraitDef> getRelTraitDefs() {
+            return ImmutableList.<RelTraitDef>of(RelCollationTraitDef.INSTANCE);
+          }
+          @Override public RelTraitSet emptyTraitSet() {
+            return RelTraitSet.createEmpty().plus(
+                RelCollationTraitDef.INSTANCE.getDefault());
+          }
+        };
+      }
+    };
+
+    // Run query and save plan after trimming
+    final String sql = "select count(a.EMPNO)\n"
+        + "from (select * from emp order by sal limit 3) a\n"
+        + "where a.EMPNO > 10 group by 2";
+    RelNode afterTrim = tester.convertSqlToRel(sql).rel;
+
+    // Get Sort and Filter operators
+    final List<RelNode> rels = new ArrayList<>();
+    final RelShuttleImpl visitor = new RelShuttleImpl() {
+      @Override public RelNode visit(LogicalSort sort) {
+        rels.add(sort);
+        return super.visit(sort);
+      }
+      @Override public RelNode visit(LogicalFilter filter) {
+        rels.add(filter);
+        return super.visit(filter);
+      }
+    };
+    visitor.visit(afterTrim);
+
+    // Ensure sort and filter operators have consistent traitSet after trimming
+    assertThat(rels.size(), is(2));
+    RelTrait filterCollation = rels.get(0).getTraitSet()
+        .getTrait(RelCollationTraitDef.INSTANCE);
+    RelTrait sortCollation = rels.get(1).getTraitSet()
+        .getTrait(RelCollationTraitDef.INSTANCE);
+    assertTrue(filterCollation.satisfies(sortCollation));
+  }
+
   @Test public void testOffset0() {
     final String sql = "select * from emp offset 0";
     sql(sql).ok();
diff --git a/core/src/test/resources/sql/misc.iq b/core/src/test/resources/sql/misc.iq
index 78a6c1c..a7127ac 100644
--- a/core/src/test/resources/sql/misc.iq
+++ b/core/src/test/resources/sql/misc.iq
@@ -355,7 +355,7 @@ where not exists (select 1 from "hr"."emps");
 (0 rows)
 
 !ok
-EnumerableCalc(expr#0..1=[{inputs}], expr#2=[IS NOT NULL($t1)], expr#3=[NOT($t2)], deptno=[$t0], $condition=[$t3])
+EnumerableCalc(expr#0..1=[{inputs}], expr#2=[IS NULL($t1)], deptno=[$t0], $condition=[$t2])
   EnumerableHashJoin(condition=[true], joinType=[left])
     EnumerableCalc(expr#0..3=[{inputs}], deptno=[$t0])
       EnumerableTableScan(table=[[hr, depts]])
@@ -397,7 +397,7 @@ where not exists (select 1 from "hr"."emps" where "empid" < 0);
 (3 rows)
 
 !ok
-EnumerableCalc(expr#0..1=[{inputs}], expr#2=[IS NOT NULL($t1)], expr#3=[NOT($t2)], deptno=[$t0], $condition=[$t3])
+EnumerableCalc(expr#0..1=[{inputs}], expr#2=[IS NULL($t1)], deptno=[$t0], $condition=[$t2])
   EnumerableHashJoin(condition=[true], joinType=[left])
     EnumerableCalc(expr#0..3=[{inputs}], deptno=[$t0])
       EnumerableTableScan(table=[[hr, depts]])
diff --git a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java
index 7b7433e..67c2e6e 100644
--- a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java
+++ b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java
@@ -323,11 +323,11 @@ public class GeodeAllDataTypesTest extends AbstractGeodeTest {
         .queryContains(
             GeodeAssertions.query("SELECT stringValue AS stringValue "
                 + "FROM /allDataTypesRegion WHERE "
-                + "stringValue IN SET('abc', 'def') OR floatValue IN SET(1.5678, null) OR dateValue "
+                + "stringValue IN SET('abc', 'def') OR floatValue = 1.5678 OR dateValue "
                 + "IN SET(DATE '2018-02-05', DATE '2018-02-06') OR timeValue "
                 + "IN SET(TIME '03:22:23', TIME '07:22:23') OR timestampValue "
                 + "IN SET(TIMESTAMP '2018-02-05 04:22:33', TIMESTAMP '2017-02-05 04:22:33') "
-                + "OR booleanValue IN SET(true, false, null)"));
+                + "OR booleanValue = true OR booleanValue = false"));
   }
 }
 
diff --git a/pig/src/main/java/org/apache/calcite/adapter/pig/PigRelFactories.java b/pig/src/main/java/org/apache/calcite/adapter/pig/PigRelFactories.java
index eaeed00..c64d1fb 100644
--- a/pig/src/main/java/org/apache/calcite/adapter/pig/PigRelFactories.java
+++ b/pig/src/main/java/org/apache/calcite/adapter/pig/PigRelFactories.java
@@ -20,6 +20,7 @@ import org.apache.calcite.plan.Context;
 import org.apache.calcite.plan.Contexts;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.plan.RelTraitSet;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.AggregateCall;
 import org.apache.calcite.rel.core.CorrelationId;
@@ -28,6 +29,7 @@ import org.apache.calcite.rel.core.RelFactories;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.util.ImmutableBitSet;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 
 import java.util.List;
@@ -70,9 +72,13 @@ public class PigRelFactories {
 
     public static final PigFilterFactory INSTANCE = new PigFilterFactory();
 
-    @Override public RelNode createFilter(RelNode input, RexNode condition) {
-      return new PigFilter(input.getCluster(), input.getTraitSet().replace(PigRel.CONVENTION),
-          input, condition);
+    @Override public RelNode createFilter(RelNode input, RexNode condition,
+        Set<CorrelationId> variablesSet) {
+      Preconditions.checkArgument(variablesSet.isEmpty(),
+          "PigFilter does not allow variables");
+      final RelTraitSet traitSet =
+          input.getTraitSet().replace(PigRel.CONVENTION);
+      return new PigFilter(input.getCluster(), traitSet, input, condition);
     }
   }
 
diff --git a/site/_docs/algebra.md b/site/_docs/algebra.md
index ba07531..f8294d7 100644
--- a/site/_docs/algebra.md
+++ b/site/_docs/algebra.md
@@ -309,7 +309,7 @@ return the `RelBuilder`.
 | `functionScan(operator, n, expr...)`<br/>`functionScan(operator, n, exprList)` | Creates a [TableFunctionScan]({{ site.apiRoot }}/org/apache/calcite/rel/core/TableFunctionScan.html) of the `n` most recent relational expressions.
 | `transientScan(tableName [, rowType])` | Creates a [TableScan]({{ site.apiRoot }}/org/apache/calcite/rel/core/TableScan.html) on a [TransientTable]]({{ site.apiRoot }}/org/apache/calcite/schema/TransientTable.html) with the given type (if not specified, the most recent relational expression's type will be used).
 | `values(fieldNames, value...)`<br/>`values(rowType, tupleList)` | Creates a [Values]({{ site.apiRoot }}/org/apache/calcite/rel/core/Values.html).
-| `filter(expr...)`<br/>`filter(exprList)` | Creates a [Filter]({{ site.apiRoot }}/org/apache/calcite/rel/core/Filter.html) over the AND of the given predicates.
+| `filter([variablesSet, ] exprList)`<br/>`filter([variablesSet, ] expr...)` | Creates a [Filter]({{ site.apiRoot }}/org/apache/calcite/rel/core/Filter.html) over the AND of the given predicates; if `variablesSet` is specified, the predicates may reference those variables.
 | `project(expr...)`<br/>`project(exprList [, fieldNames])` | Creates a [Project]({{ site.apiRoot }}/org/apache/calcite/rel/core/Project.html). To override the default name, wrap expressions using `alias`, or specify the `fieldNames` argument.
 | `projectPlus(expr...)`<br/>`projectPlus(exprList)` | Variant of `project` that keeps original fields and appends the given expressions.
 | `permute(mapping)` | Creates a [Project]({{ site.apiRoot }}/org/apache/calcite/rel/core/Project.html) that permutes the fields using `mapping`.
@@ -350,6 +350,8 @@ Argument types:
 * `tupleList` Iterable of List of [RexLiteral]({{ site.apiRoot }}/org/apache/calcite/rex/RexLiteral.html)
 * `all`, `distinct`, `strictStart`, `strictEnd`, `allRows` boolean
 * `alias` String
+* `variablesSet` Iterable of
+  [CorrelationId]({{ site.apiRoot }}/org/apache/calcite/rel/core/CorrelationId.html)
 * `varHolder` [Holder]({{ site.apiRoot }}/org/apache/calcite/util/Holder.html) of [RexCorrelVariable]({{ site.apiRoot }}/org/apache/calcite/rex/RexCorrelVariable.html)
 * `patterns` Map whose key is String, value is [RexNode]({{ site.apiRoot }}/org/apache/calcite/rex/RexNode.html)
 * `subsets` Map whose key is String, value is a sorted set of String