Repository: kudu Updated Branches: refs/heads/branch-1.0.x 40637ef8f -> d2ebf5aa7
KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client Part 1 fixed the C++ client/server and Java client so that specifying a an IS NOT NULL predicate on a non-nullable column would not result in a CHECK failure or exception. This commit filters all such predicates from new scan requests on the client, in order to avoid crashing server versions without the part 1 fix (1.0.0 and below). This is a difficult thing to test, since the underlying issue has already been fixed, and this just prevents older server version from crashing. I've manually verified the change by testing this patch with the additional tests in part 1, but without the fix from part 1. Change-Id: I542a735ca337ec9ed4eb3c989f8e6fa4ee0cc1da Reviewed-on: http://gerrit.cloudera.org:8080/4612 Reviewed-by: Jean-Daniel Cryans <[email protected]> Reviewed-by: Adar Dembo <[email protected]> Tested-by: Dan Burkert <[email protected]> Reviewed-on: http://gerrit.cloudera.org:8080/4619 Reviewed-by: Dan Burkert <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d2ebf5aa Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d2ebf5aa Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d2ebf5aa Branch: refs/heads/branch-1.0.x Commit: d2ebf5aa7e5165ffd12c3e692547455c7888363a Parents: 40637ef Author: Dan Burkert <[email protected]> Authored: Mon Oct 3 16:06:34 2016 -0700 Committer: Dan Burkert <[email protected]> Committed: Tue Oct 4 19:53:02 2016 +0000 ---------------------------------------------------------------------- .../kudu/client/AbstractKuduScannerBuilder.java | 14 ++++++++++---- src/kudu/common/scan_spec-test.cc | 10 ++++++---- src/kudu/common/scan_spec.cc | 10 ++++++++++ 3 files changed, 26 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/d2ebf5aa/java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java ---------------------------------------------------------------------- diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java index 17bc66d..7e913cf 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java @@ -126,11 +126,17 @@ public abstract class AbstractKuduScannerBuilder public S addPredicate(KuduPredicate predicate) { String columnName = predicate.getColumn().getName(); KuduPredicate existing = predicates.get(columnName); - if (existing == null) { - predicates.put(columnName, predicate); - } else { - predicates.put(columnName, existing.merge(predicate)); + if (existing != null) { + predicate = existing.merge(predicate); + } + + // KUDU-1652: Do not send an IS NOT NULL predicate to the server for a non-nullable column. + if (!predicate.getColumn().isNullable() && + predicate.getType() == KuduPredicate.PredicateType.IS_NOT_NULL) { + return (S) this; } + + predicates.put(columnName, predicate); return (S) this; } http://git-wip-us.apache.org/repos/asf/kudu/blob/d2ebf5aa/src/kudu/common/scan_spec-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/common/scan_spec-test.cc b/src/kudu/common/scan_spec-test.cc index 1bfa404..756afe5 100644 --- a/src/kudu/common/scan_spec-test.cc +++ b/src/kudu/common/scan_spec-test.cc @@ -103,7 +103,8 @@ class CompositeIntKeysTest : public TestScanSpec { TestScanSpec( Schema({ ColumnSchema("a", INT8), ColumnSchema("b", INT8), - ColumnSchema("c", INT8) }, + ColumnSchema("c", INT8), + ColumnSchema("d", INT8, true) }, 3)) { } }; @@ -309,15 +310,16 @@ TEST_F(CompositeIntKeysTest, TestPredicateOrderDoesntMatter) { spec.ToString(schema_)); } -// Test that IS NOT NULL predicates do *not* get pushed into the primary key -// bounds. This is a regression test for KUDU-1652, where previously attempting +// Test that IS NOT NULL predicates do *not* get filtered from non-nullable +// columns. This is a regression test for KUDU-1652, where previously attempting // to push an IS NOT NULL predicate would cause a CHECK failure. TEST_F(CompositeIntKeysTest, TestIsNotNullPushdown) { ScanSpec spec; spec.AddPredicate(ColumnPredicate::IsNotNull(schema_.column(0))); + spec.AddPredicate(ColumnPredicate::IsNotNull(schema_.column(3))); SCOPED_TRACE(spec.ToString(schema_)); spec.OptimizeScan(schema_, &arena_, &pool_, true); - EXPECT_EQ("`a` IS NOT NULL", spec.ToString(schema_)); + EXPECT_EQ("`d` IS NOT NULL", spec.ToString(schema_)); } // Tests that a scan spec without primary key bounds will not have predicates http://git-wip-us.apache.org/repos/asf/kudu/blob/d2ebf5aa/src/kudu/common/scan_spec.cc ---------------------------------------------------------------------- diff --git a/src/kudu/common/scan_spec.cc b/src/kudu/common/scan_spec.cc index d8469e0..7655035 100644 --- a/src/kudu/common/scan_spec.cc +++ b/src/kudu/common/scan_spec.cc @@ -124,6 +124,16 @@ void ScanSpec::OptimizeScan(const Schema& schema, LiftPrimaryKeyBounds(schema, arena); PushPredicatesIntoPrimaryKeyBounds(schema, arena, pool, remove_pushed_predicates); } + + // KUDU-1652: Filter IS NOT NULL predicates for non-nullable columns. + for (auto itr = predicates_.begin(); itr != predicates_.end(); ) { + if (!itr->second.column().is_nullable() && + itr->second.predicate_type() == PredicateType::IsNotNull) { + itr = predicates_.erase(itr); + } else { + itr = std::next(itr); + } + } } void ScanSpec::PushPredicatesIntoPrimaryKeyBounds(const Schema& schema,
