Repository: kudu Updated Branches: refs/heads/master 6d696bcf7 -> 1a60ed7d2
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 <jdcry...@apache.org> Reviewed-by: Adar Dembo <a...@cloudera.com> Tested-by: Dan Burkert <d...@cloudera.com> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/624a88b1 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/624a88b1 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/624a88b1 Branch: refs/heads/master Commit: 624a88b18718ea0abc39e32923bb48339398a429 Parents: 6d696bc Author: Dan Burkert <d...@cloudera.com> Authored: Mon Oct 3 16:06:34 2016 -0700 Committer: Dan Burkert <d...@cloudera.com> Committed: Tue Oct 4 19:42:54 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/624a88b1/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 ae7b5d4..c23e07e 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/624a88b1/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 79f6486..10ac42f 100644 --- a/src/kudu/common/scan_spec-test.cc +++ b/src/kudu/common/scan_spec-test.cc @@ -118,7 +118,8 @@ class CompositeIntKeysTest : public TestScanSpec { TestScanSpec( Schema({ ColumnSchema("a", INT8), ColumnSchema("b", INT8), - ColumnSchema("c", INT8) }, + ColumnSchema("c", INT8), + ColumnSchema("d", INT8, true) }, 3)) { } }; @@ -324,15 +325,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_)); } // Test that IN list predicates get pushed into the primary key bounds. http://git-wip-us.apache.org/repos/asf/kudu/blob/624a88b1/src/kudu/common/scan_spec.cc ---------------------------------------------------------------------- diff --git a/src/kudu/common/scan_spec.cc b/src/kudu/common/scan_spec.cc index 7beea46..3f3ad07 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,
