You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2017/02/01 21:25:42 UTC
[1/3] kudu git commit: openssl_util: avoid non-POD vector in static
storage
Repository: kudu
Updated Branches:
refs/heads/master 63ce3ec63 -> 86714152d
openssl_util: avoid non-POD vector in static storage
This addresses an ASAN crash I've seen a couple times now in test runs
during static destruction. We previously used a vector<Mutex*> for the
OpenSSL locks, but the order of static destruction isn't very easy to
predict, and the destructor of things like SSL sockets may end up
needing to acquire these locks.
This patch switches to a C-style dynamic array.
An example ASAN trace follows.
=================================================================
==28629==ERROR: AddressSanitizer: heap-use-after-free on address 0x61300000b6c0 at pc 0x7f25c7d03339 bp 0x7f25bbdc3af0 sp 0x7f25bbdc3ae8
READ of size 8 at 0x61300000b6c0 thread T4 (rpc reactor-286)
#0 0x7f25c7d03338 in kudu::security::(anonymous namespace)::LockingCB(int, int, char const*, int) /home/jenkins-slave/workspace/kudu-1/src/kudu/security/openssl_util.cc:54:14
#1 0x7f25c4c41836 in CRYPTO_add_lock (/lib/x86_64-linux-gnu/libcrypto.so.1.0.0+0x5f836)
#2 0x7f25c49bcedb in SSL_free (/lib/x86_64-linux-gnu/libssl.so.1.0.0+0x39edb)
#3 0x7f25c7d10f20 in std::_Function_handler<void (ssl_st*), void (*)(ssl_st*)>::_M_invoke(std::_Any_data const&, ssl_st*) /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/functional:2071:2
#4 0x7f25c8764ed4 in std::function<void (ssl_st*)>::operator()(ssl_st*) const /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/functional:2471:14
#5 0x7f25c7d0daf7 in std::unique_ptr<ssl_st, std::function<void (ssl_st*)> >::reset(ssl_st*) /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/unique_ptr.h:262:4
#6 0x7f25c7d14f78 in kudu::security::TlsSocket::Close() /home/jenkins-slave/workspace/kudu-1/src/kudu/security/tls_socket.cc:115:8
#7 0x7f25c8773072 in kudu::rpc::Connection::Shutdown(kudu::Status const&) /home/jenkins-slave/workspace/kudu-1/src/kudu/rpc/connection.cc:174:5
#8 0x7f25c87bfb25 in kudu::rpc::ReactorThread::ShutdownInternal() /home/jenkins-slave/workspace/kudu-1/src/kudu/rpc/reactor.cc:134:11
#9 0x7f25c87c0f3f in kudu::rpc::ReactorThread::AsyncHandler(ev::async&, int) /home/jenkins-slave/workspace/kudu-1/src/kudu/rpc/reactor.cc:198:5
...
0x61300000b6c0 is located 128 bytes inside of 328-byte region [0x61300000b640,0x61300000b788)
freed by thread T0 here:
#0 0x5c1f30 in operator delete(void*) /home/jenkins-slave/workspace/kudu-1/thirdparty/src/llvm-3.9.1.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:110
#1 0x7f25c7d05235 in std::_Vector_base<kudu::Mutex*, std::allocator<kudu::Mutex*> >::~_Vector_base() /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_vector.h:160:9
#2 0x7f25c2b30539 in __cxa_finalize /build/eglibc-oGUzwX/eglibc-2.19/stdlib/cxa_finalize.c:56
previously allocated by thread T0 here:
#0 0x5c1870 in operator new(unsigned long) /home/jenkins-slave/workspace/kudu-1/thirdparty/src/llvm-3.9.1.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:78
#1 0x7f25c7d03cfc in kudu::Mutex** std::vector<kudu::Mutex*, std::allocator<kudu::Mutex*> >::_M_allocate_and_copy<std::move_iterator<kudu::Mutex**> >(unsigned long, std::move_iterator<kudu::Mutex**>, std::move_iterator<kudu::Mutex**>) /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../
#2 0x7f25c7d038e6 in std::vector<kudu::Mutex*, std::allocator<kudu::Mutex*> >::reserve(unsigned long) /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/vector.tcc:73:20
#3 0x7f25c7d02420 in kudu::security::(anonymous namespace)::DoInitializeOpenSSL() /home/jenkins-slave/workspace/kudu-1/src/kudu/security/openssl_util.cc:78:16
...
Change-Id: Id6fdd1162eb39114c67f3c46073345829530434f
Reviewed-on: http://gerrit.cloudera.org:8080/5853
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/588538da
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/588538da
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/588538da
Branch: refs/heads/master
Commit: 588538da02c224e9b6adbd8823bc7bff421ff21a
Parents: 63ce3ec
Author: Todd Lipcon <to...@apache.org>
Authored: Tue Jan 31 21:23:33 2017 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Wed Feb 1 19:14:24 2017 +0000
----------------------------------------------------------------------
src/kudu/security/openssl_util.cc | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/588538da/src/kudu/security/openssl_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/openssl_util.cc b/src/kudu/security/openssl_util.cc
index c32cb72..9f552f8 100644
--- a/src/kudu/security/openssl_util.cc
+++ b/src/kudu/security/openssl_util.cc
@@ -22,7 +22,6 @@
#include <mutex>
#include <sstream>
#include <string>
-#include <vector>
#include <glog/logging.h>
#include <openssl/err.h>
@@ -38,7 +37,6 @@
using std::ostringstream;
using std::string;
-using std::vector;
using strings::Substitute;
namespace kudu {
@@ -46,13 +44,14 @@ namespace security {
namespace {
-vector<Mutex*> kCryptoLocks;
+// Array of locks used by OpenSSL.
+// We use an intentionally-leaked C-style array here to avoid non-POD static data.
+Mutex* kCryptoLocks = nullptr;
// Lock/Unlock the nth lock. Only to be used by OpenSSL.
void LockingCB(int mode, int type, const char* /*file*/, int /*line*/) {
- DCHECK(!kCryptoLocks.empty());
- Mutex* m = kCryptoLocks[type];
- DCHECK(m);
+ DCHECK(kCryptoLocks);
+ Mutex* m = &kCryptoLocks[type];
if (mode & CRYPTO_LOCK) {
m->lock();
} else {
@@ -75,10 +74,8 @@ void DoInitializeOpenSSL() {
// LSAN warnings.
debug::ScopedLeakCheckDisabler d;
int num_locks = CRYPTO_num_locks();
- kCryptoLocks.reserve(num_locks);
- for (int i = 0; i < num_locks; i++) {
- kCryptoLocks.emplace_back(new Mutex());
- }
+ CHECK(!kCryptoLocks);
+ kCryptoLocks = new Mutex[num_locks];
// Callbacks used by OpenSSL required in a multi-threaded setting.
CRYPTO_set_locking_callback(LockingCB);
[3/3] kudu git commit: Extract a static function to generate a
self-signed CA
Posted by to...@apache.org.
Extract a static function to generate a self-signed CA
We had some similar code in a test case and in MasterCertAuthority. This
extracts a function to create a self-signed CA, as well as a test
utility function which creates both a self-signed CA and private key.
Currently the test utility is only used from one test case, but I have
plans to use it elsewhere.
Change-Id: Ib40add99ed96e4753a3810709c66d07c96ab70bd
Reviewed-on: http://gerrit.cloudera.org:8080/5844
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Dan Burkert <da...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/86714152
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/86714152
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/86714152
Branch: refs/heads/master
Commit: 86714152d3122580984a28ad0fc46ed942d5020c
Parents: 431aee5
Author: Todd Lipcon <to...@apache.org>
Authored: Tue Jan 31 14:52:37 2017 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Wed Feb 1 21:24:31 2017 +0000
----------------------------------------------------------------------
src/kudu/master/master_cert_authority.cc | 21 ++-------
src/kudu/security/CMakeLists.txt | 1 +
src/kudu/security/ca/cert_management-test.cc | 27 +++---------
src/kudu/security/ca/cert_management.cc | 20 +++++++++
src/kudu/security/ca/cert_management.h | 34 +++++++++------
src/kudu/security/security-test-util.cc | 52 +++++++++++++++++++++++
src/kudu/security/security-test-util.h | 11 ++++-
7 files changed, 112 insertions(+), 54 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/86714152/src/kudu/master/master_cert_authority.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master_cert_authority.cc b/src/kudu/master/master_cert_authority.cc
index 8b33991..45190a1 100644
--- a/src/kudu/master/master_cert_authority.cc
+++ b/src/kudu/master/master_cert_authority.cc
@@ -75,26 +75,13 @@ MasterCertAuthority::~MasterCertAuthority() {
Status MasterCertAuthority::Init() {
CHECK(!ca_private_key_);
- // Create a key for the self-signed CA.
- shared_ptr<PrivateKey> key(make_shared<PrivateKey>());
+ // Create a key and cert for the self-signed CA.
+ auto key = make_shared<PrivateKey>();
+ auto ca_cert = make_shared<Cert>();
RETURN_NOT_OK(GeneratePrivateKey(FLAGS_master_ca_rsa_key_length_bits,
key.get()));
- // Generate a CSR for the CA.
- CertSignRequest ca_csr;
- {
- CaCertRequestGenerator gen(PrepareCaConfig(server_uuid_));
- RETURN_NOT_OK(gen.Init());
- RETURN_NOT_OK(gen.GenerateRequest(*key, &ca_csr));
- }
-
- // Self-sign the CA's CSR.
- auto ca_cert = make_shared<Cert>();
- {
- CertSigner ca_signer;
- RETURN_NOT_OK(ca_signer.InitForSelfSigning(key));
- RETURN_NOT_OK(ca_signer.Sign(ca_csr, ca_cert.get()));
- }
+ RETURN_NOT_OK(CertSigner::SelfSignCA(key, PrepareCaConfig(server_uuid_), ca_cert.get()));
// Initialize our signer with the new CA.
auto signer = make_shared<CertSigner>();
http://git-wip-us.apache.org/repos/asf/kudu/blob/86714152/src/kudu/security/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/security/CMakeLists.txt b/src/kudu/security/CMakeLists.txt
index 2627b77..4a4501f 100644
--- a/src/kudu/security/CMakeLists.txt
+++ b/src/kudu/security/CMakeLists.txt
@@ -90,6 +90,7 @@ ADD_EXPORTABLE_LIBRARY(security
if (NOT NO_TESTS)
set(SECURITY_TEST_SRCS
+ security-test-util.cc
test/mini_kdc.cc
test/test_certs.cc)
http://git-wip-us.apache.org/repos/asf/kudu/blob/86714152/src/kudu/security/ca/cert_management-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/ca/cert_management-test.cc b/src/kudu/security/ca/cert_management-test.cc
index c4d9084..39dfec8 100644
--- a/src/kudu/security/ca/cert_management-test.cc
+++ b/src/kudu/security/ca/cert_management-test.cc
@@ -17,6 +17,7 @@
#include "kudu/security/ca/cert_management.h"
+#include <memory>
#include <thread>
#include <utility>
#include <vector>
@@ -26,6 +27,7 @@
#include "kudu/gutil/strings/util.h"
#include "kudu/security/cert.h"
#include "kudu/security/openssl_util.h"
+#include "kudu/security/security-test-util.h"
#include "kudu/security/test/test_certs.h"
#include "kudu/util/env.h"
#include "kudu/util/path_util.h"
@@ -33,6 +35,7 @@
#include "kudu/util/test_macros.h"
#include "kudu/util/test_util.h"
+using std::shared_ptr;
using std::string;
using std::vector;
using std::thread;
@@ -372,27 +375,9 @@ TEST_F(CertManagementTest, SignCaCert) {
// Test the creation and use of a CA which uses a self-signed CA cert
// generated on the fly.
TEST_F(CertManagementTest, TestSelfSignedCA) {
- // Create a key for the self-signed CA.
- auto ca_key = std::make_shared<PrivateKey>();
- ASSERT_OK(GeneratePrivateKey(2048, ca_key.get()));
-
- // Generate a CSR for the CA.
- CertSignRequest ca_csr;
- {
- const CertRequestGenerator::Config gen_config(
- PrepareConfig("8C084CF6-A30B-4F5B-9673-A73E62E29A9D"));
- CaCertRequestGenerator gen(gen_config);
- ASSERT_OK(gen.Init());
- ASSERT_OK(gen.GenerateRequest(*ca_key, &ca_csr));
- }
-
- // Self-sign the CA's CSR.
- auto ca_cert = std::make_shared<Cert>();
- {
- CertSigner ca_signer;
- ASSERT_OK(ca_signer.InitForSelfSigning(ca_key));
- ASSERT_OK(ca_signer.Sign(ca_csr, ca_cert.get()));
- }
+ shared_ptr<PrivateKey> ca_key;
+ shared_ptr<Cert> ca_cert;
+ ASSERT_OK(GenerateSelfSignedCAForTests(&ca_key, &ca_cert));
// Create a key for the tablet server.
auto ts_key = std::make_shared<PrivateKey>();
http://git-wip-us.apache.org/repos/asf/kudu/blob/86714152/src/kudu/security/ca/cert_management.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/ca/cert_management.cc b/src/kudu/security/ca/cert_management.cc
index 8de37dd..710b23e 100644
--- a/src/kudu/security/ca/cert_management.cc
+++ b/src/kudu/security/ca/cert_management.cc
@@ -281,6 +281,26 @@ Status CaCertRequestGenerator::SetExtensions(X509_REQ* req) const {
return Status::OK();
}
+Status CertSigner::SelfSignCA(const shared_ptr<PrivateKey>& key,
+ CaCertRequestGenerator::Config config,
+ Cert* cert) {
+ // Generate a CSR for the CA.
+ CertSignRequest ca_csr;
+ {
+ CaCertRequestGenerator gen(std::move(config));
+ RETURN_NOT_OK(gen.Init());
+ RETURN_NOT_OK(gen.GenerateRequest(*key, &ca_csr));
+ }
+
+ // Self-sign the CA's CSR.
+ {
+ CertSigner ca_signer;
+ RETURN_NOT_OK(ca_signer.InitForSelfSigning(key));
+ RETURN_NOT_OK(ca_signer.Sign(ca_csr, cert));
+ }
+ return Status::OK();
+}
+
CertSigner::CertSigner()
: is_initialized_(false) {
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/86714152/src/kudu/security/ca/cert_management.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/ca/cert_management.h b/src/kudu/security/ca/cert_management.h
index 4974983..7a2887a 100644
--- a/src/kudu/security/ca/cert_management.h
+++ b/src/kudu/security/ca/cert_management.h
@@ -54,15 +54,15 @@ class CertRequestGeneratorBase {
// Properties for the generated X509 CSR. Using server UUID for the common
// name field.
struct Config {
- const std::string country; // subject field: C
- const std::string state; // subject field: ST
- const std::string locality; // subject field: L
- const std::string org; // subject field: O
- const std::string unit; // subject field: OU
- const std::string uuid; // subject field: CN
- const std::string comment; // custom extension: Netscape Comment
- const std::vector<std::string> hostnames; // subjectAltName extension (DNS:)
- const std::vector<std::string> ips; // subjectAltName extension (IP:)
+ std::string country; // subject field: C
+ std::string state; // subject field: ST
+ std::string locality; // subject field: L
+ std::string org; // subject field: O
+ std::string unit; // subject field: OU
+ std::string uuid; // subject field: CN
+ std::string comment; // custom extension: Netscape Comment
+ std::vector<std::string> hostnames; // subjectAltName extension (DNS:)
+ std::vector<std::string> ips; // subjectAltName extension (IP:)
};
explicit CertRequestGeneratorBase(Config config);
@@ -134,6 +134,11 @@ class CaCertRequestGenerator : public CertRequestGeneratorBase {
// An utility class for issuing and signing certificates.
class CertSigner {
public:
+ // Generate a self-signed certificate authority using the given key
+ // and CSR configuration.
+ static Status SelfSignCA(const std::shared_ptr<PrivateKey>& key,
+ CaCertRequestGenerator::Config config,
+ Cert* cert);
// Create a CertSigner.
// Exactly one of the following Init*() methods must be called
// exactly once before the instance may be used.
@@ -149,11 +154,6 @@ class CertSigner {
// on disk.
Status InitFromFiles(const std::string& ca_cert_path,
const std::string& ca_private_key_path);
- // Initialize the signer for self-signing using the given private key.
- //
- // Any certificates signed by this CertSigner will have the 'issuer' equal
- // to the signed cert's subject.
- Status InitForSelfSigning(std::shared_ptr<PrivateKey> private_key);
// Set the expiration interval for certs signed by this signer.
// This may be changed at any point.
@@ -175,6 +175,12 @@ class CertSigner {
static Status DigestSign(const EVP_MD* md, EVP_PKEY* pkey, X509* x);
static Status GenerateSerial(c_unique_ptr<ASN1_INTEGER>* ret);
+ // Initialize the signer for self-signing using the given private key.
+ //
+ // Any certificates signed by this CertSigner will have the 'issuer' equal
+ // to the signed cert's subject.
+ Status InitForSelfSigning(std::shared_ptr<PrivateKey> private_key);
+
Status DoSign(const EVP_MD* digest, int32_t exp_seconds, X509 *ret) const;
mutable simple_spinlock lock_;
http://git-wip-us.apache.org/repos/asf/kudu/blob/86714152/src/kudu/security/security-test-util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/security-test-util.cc b/src/kudu/security/security-test-util.cc
new file mode 100644
index 0000000..2fa8b1f
--- /dev/null
+++ b/src/kudu/security/security-test-util.cc
@@ -0,0 +1,52 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "kudu/security/security-test-util.h"
+
+#include <memory>
+
+#include <glog/logging.h>
+
+#include "kudu/security/ca/cert_management.h"
+#include "kudu/security/cert.h"
+#include "kudu/security/crypto.h"
+
+namespace kudu {
+namespace security {
+
+using ca::CaCertRequestGenerator;
+using ca::CertSigner;
+
+Status GenerateSelfSignedCAForTests(std::shared_ptr<PrivateKey>* ca_key,
+ std::shared_ptr<Cert>* ca_cert) {
+ // Create a key for the self-signed CA.
+ auto ret_ca_key = std::make_shared<PrivateKey>();
+ auto ret_ca_cert = std::make_shared<Cert>();
+ RETURN_NOT_OK(GeneratePrivateKey(512, ret_ca_key.get()));
+
+ CaCertRequestGenerator::Config config;
+ config.uuid = "test-ca-uuid";
+ RETURN_NOT_OK(CertSigner::SelfSignCA(ret_ca_key, config, ret_ca_cert.get()));
+
+ *ca_key = std::move(ret_ca_key);
+ *ca_cert = std::move(ret_ca_cert);
+ return Status::OK();
+}
+
+
+} // namespace security
+} // namespace kudu
http://git-wip-us.apache.org/repos/asf/kudu/blob/86714152/src/kudu/security/security-test-util.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/security-test-util.h b/src/kudu/security/security-test-util.h
index 76853d4..17c5375 100644
--- a/src/kudu/security/security-test-util.h
+++ b/src/kudu/security/security-test-util.h
@@ -25,10 +25,13 @@
namespace kudu {
namespace security {
+class Cert;
+class PrivateKey;
+
// TODO(todd): consolidate these certs with those in
// security/test/test_certs.h once we support configuring a password
// for the RPC cert.
-static Status CreateSSLServerCert(const std::string& file_path) {
+inline static Status CreateSSLServerCert(const std::string& file_path) {
static const char* test_server_cert = R"(
-----BEGIN CERTIFICATE-----
MIIEejCCA2KgAwIBAgIJAKMdvDR5PL82MA0GCSqGSIb3DQEBBQUAMIGEMQswCQYD
@@ -62,7 +65,7 @@ seCrQwgi1Fer9Ekd5XNjWjigC3VC3SjMqWaxeKbZ2/AuABJMz5xSiRkgwphXEQ==
}
// Writes the test SSL private key into a temporary file.
-static Status CreateSSLPrivateKey(const std::string& file_path) {
+inline static Status CreateSSLPrivateKey(const std::string& file_path) {
static const char* test_private_key = R"(
-----BEGIN RSA PRIVATE KEY-----
MIIEpAIBAAKCAQEAqFI96TENhC5K886vpKIsZY1RQQBBKsFFkowPhhCsHxW/1D0Y
@@ -96,5 +99,9 @@ dc+JVPKL8Fe4a8fmsI6ndcZQ9qpOdZM5WOD0ldKRc+SsrYKkTmOOJQ==
return Status::OK();
}
+// TODO(todd): change these from shared_ptrs to unique_ptrs
+Status GenerateSelfSignedCAForTests(std::shared_ptr<PrivateKey>* ca_key,
+ std::shared_ptr<Cert>* ca_cert);
+
} // namespace security
} // namespace kudu
[2/3] kudu git commit: KUDU-1595 and KUDU-1642: IS NOT NULL and IS
NULL predicates
Posted by to...@apache.org.
KUDU-1595 and KUDU-1642: IS NOT NULL and IS NULL predicates
This patch exposes client APIs for the IS NOT NULL predicate and
adds an IS NULL predicate type.
These are both purely new features, so the changes are backwards-compatible.
Change-Id: Ide23ab6d3cad524553aa6ea5c44570fdadde5ace
Reviewed-on: http://gerrit.cloudera.org:8080/5439
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/431aee53
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/431aee53
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/431aee53
Branch: refs/heads/master
Commit: 431aee53fd269b5699095fb1e7fd5402644fe454
Parents: 588538d
Author: Will Berkeley <wd...@gmail.com>
Authored: Fri Dec 9 02:39:43 2016 -0500
Committer: Will Berkeley <wd...@gmail.com>
Committed: Wed Feb 1 20:42:21 2017 +0000
----------------------------------------------------------------------
.../org/apache/kudu/client/KuduPredicate.java | 81 +++++---
.../org/apache/kudu/client/TestKuduClient.java | 21 +-
.../apache/kudu/client/TestKuduPredicate.java | 204 ++++++++++++++++---
.../apache/kudu/client/TestPartitionPruner.java | 6 +-
.../apache/kudu/client/TestScanPredicate.java | 30 +++
src/kudu/cfile/binary_plain_block.cc | 1 +
src/kudu/client/client-test.cc | 193 ++++++++++++++++++
src/kudu/client/client.cc | 63 +++---
src/kudu/client/client.h | 24 +++
src/kudu/client/predicate-test.cc | 41 +++-
src/kudu/client/scan_predicate-internal.h | 44 ++++
src/kudu/client/scan_predicate.h | 2 +
src/kudu/client/scan_token-internal.cc | 4 +
src/kudu/client/scan_token-test.cc | 52 +++++
src/kudu/client/table-internal.h | 18 ++
src/kudu/common/column_predicate-test.cc | 80 +++++++-
src/kudu/common/column_predicate.cc | 100 +++++++--
src/kudu/common/column_predicate.h | 23 ++-
src/kudu/common/common.proto | 3 +
src/kudu/common/key_util.cc | 6 +-
src/kudu/common/scan_spec.cc | 1 +
src/kudu/common/wire_protocol.cc | 9 +-
22 files changed, 897 insertions(+), 109 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
index ae404b4..3265d19 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
@@ -59,6 +59,8 @@ public class KuduPredicate {
RANGE,
/** A predicate which filters all null rows. */
IS_NOT_NULL,
+ /** A predicate which filters all non-null rows. */
+ IS_NULL,
/** A predicate which filters all rows not matching a list of values. */
IN_LIST,
}
@@ -120,7 +122,7 @@ public class KuduPredicate {
if (value) {
return new KuduPredicate(PredicateType.EQUALITY, column, Bytes.fromBoolean(true), null);
} else {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
}
}
case EQUAL: return new KuduPredicate(PredicateType.EQUALITY, column,
@@ -138,7 +140,7 @@ public class KuduPredicate {
// b <= true -> b IS NOT NULL
// b <= false -> b = false
if (value) {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
} else {
return new KuduPredicate(PredicateType.EQUALITY, column, Bytes.fromBoolean(false), null);
}
@@ -170,7 +172,7 @@ public class KuduPredicate {
// This has the same effect as an inclusive upper bound on the maximum
// value. If the column is not nullable then the IS NOT NULL predicate
// is ignored.
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
}
value += 1;
op = ComparisonOp.LESS;
@@ -207,7 +209,7 @@ public class KuduPredicate {
switch (op) {
case GREATER_EQUAL:
if (value == minValue) {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
} else if (value == maxValue) {
return new KuduPredicate(PredicateType.EQUALITY, column, bytes, null);
}
@@ -236,7 +238,7 @@ public class KuduPredicate {
checkColumn(column, Type.FLOAT);
if (op == ComparisonOp.LESS_EQUAL) {
if (value == Float.POSITIVE_INFINITY) {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
}
value = Math.nextAfter(value, Float.POSITIVE_INFINITY);
op = ComparisonOp.LESS;
@@ -252,7 +254,7 @@ public class KuduPredicate {
switch (op) {
case GREATER_EQUAL:
if (value == Float.NEGATIVE_INFINITY) {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
} else if (value == Float.POSITIVE_INFINITY) {
return new KuduPredicate(PredicateType.EQUALITY, column, bytes, null);
}
@@ -281,7 +283,7 @@ public class KuduPredicate {
checkColumn(column, Type.DOUBLE);
if (op == ComparisonOp.LESS_EQUAL) {
if (value == Double.POSITIVE_INFINITY) {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
}
value = Math.nextAfter(value, Double.POSITIVE_INFINITY);
op = ComparisonOp.LESS;
@@ -297,7 +299,7 @@ public class KuduPredicate {
switch (op) {
case GREATER_EQUAL:
if (value == Double.NEGATIVE_INFINITY) {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
} else if (value == Double.POSITIVE_INFINITY) {
return new KuduPredicate(PredicateType.EQUALITY, column, bytes, null);
}
@@ -337,7 +339,7 @@ public class KuduPredicate {
switch (op) {
case GREATER_EQUAL:
if (bytes.length == 0) {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
}
return new KuduPredicate(PredicateType.RANGE, column, bytes, null);
case EQUAL:
@@ -374,7 +376,7 @@ public class KuduPredicate {
switch (op) {
case GREATER_EQUAL:
if (value.length == 0) {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
}
return new KuduPredicate(PredicateType.RANGE, column, value, null);
case EQUAL:
@@ -466,6 +468,29 @@ public class KuduPredicate {
}
/**
+ * Creates a new {@code IS NOT NULL} predicate.
+ *
+ * @param column the column that the predicate applies to
+ * @return an {@code IS NOT NULL} predicate
+ */
+ public static KuduPredicate newIsNotNullPredicate(ColumnSchema column) {
+ return new KuduPredicate(PredicateType.IS_NOT_NULL, column, null, null);
+ }
+
+ /**
+ * Creates a new {@code IS NULL} predicate.
+ *
+ * @param column the column that the predicate applies to
+ * @return an {@code IS NULL} predicate
+ */
+ public static KuduPredicate newIsNullPredicate(ColumnSchema column) {
+ if (!column.isNullable()) {
+ return none(column);
+ }
+ return new KuduPredicate(PredicateType.IS_NULL, column, null, null);
+ }
+
+ /**
* @param type the predicate type
* @param column the column to which the predicate applies
* @param lower the lower bound serialized value if this is a Range predicate,
@@ -505,16 +530,6 @@ public class KuduPredicate {
}
/**
- * Factory function for an {@code IS NOT NULL} predicate.
- * @param column the column to which the predicate applies
- * @return a {@code IS NOT NULL} predicate
- */
- @VisibleForTesting
- static KuduPredicate isNotNull(ColumnSchema column) {
- return new KuduPredicate(PredicateType.IS_NOT_NULL, column, null, null);
- }
-
- /**
* @return the type of this predicate
*/
PredicateType getType() {
@@ -532,21 +547,28 @@ public class KuduPredicate {
Preconditions.checkArgument(column.equals(other.column),
"predicates from different columns may not be merged");
+ // First, consider other.type == NONE, IS_NOT_NULL, or IS_NULL
// NONE predicates dominate.
if (other.type == PredicateType.NONE) {
return other;
}
- // NOT NULL is dominated by all other predicates.
- // Note: this will no longer be true when an IS NULL predicate type is
- // added.
+ // NOT NULL is dominated by all other predicates,
+ // except IS NULL, for which the merge is NONE.
if (other.type == PredicateType.IS_NOT_NULL) {
- return this;
+ return type == PredicateType.IS_NULL ? none(column) : this;
+ }
+
+ // NULL merged with any predicate type besides itself is NONE.
+ if (other.type == PredicateType.IS_NULL) {
+ return type == PredicateType.IS_NULL ? this : none(column);
}
+ // Now other.type == EQUALITY, RANGE, or IN_LIST.
switch (type) {
case NONE: return this;
case IS_NOT_NULL: return other;
+ case IS_NULL: return none(column);
case EQUALITY: {
if (other.type == PredicateType.EQUALITY) {
if (compare(column, lower, other.lower) != 0) {
@@ -626,7 +648,7 @@ public class KuduPredicate {
private static KuduPredicate buildInList(ColumnSchema column, Collection<byte[]> values) {
// IN (true, false) predicates can be simplified to IS NOT NULL.
if (column.getType().getDataType() == Common.DataType.BOOL && values.size() > 1) {
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
}
switch (values.size()) {
@@ -695,6 +717,10 @@ public class KuduPredicate {
builder.setIsNotNull(builder.getIsNotNullBuilder());
break;
}
+ case IS_NULL: {
+ builder.setIsNull(builder.getIsNullBuilder());
+ break;
+ }
case IN_LIST: {
Common.ColumnPredicatePB.InList.Builder inListBuilder = builder.getInListBuilder();
for (byte[] value : inListValues) {
@@ -728,7 +754,9 @@ public class KuduPredicate {
range.hasUpper() ? range.getUpper().toByteArray() : null);
}
case IS_NOT_NULL:
- return isNotNull(column);
+ return newIsNotNullPredicate(column);
+ case IS_NULL:
+ return newIsNullPredicate(column);
case IN_LIST: {
Common.ColumnPredicatePB.InList inList = pb.getInList();
@@ -971,6 +999,7 @@ public class KuduPredicate {
return String.format("`%s` IN (%s)", column.getName(), Joiner.on(", ").join(strings));
}
case IS_NOT_NULL: return String.format("`%s` IS NOT NULL", column.getName());
+ case IS_NULL: return String.format("`%s` IS NULL", column.getName());
case NONE: return String.format("`%s` NONE", column.getName());
default: throw new IllegalArgumentException(String.format("unknown predicate type %s", type));
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
index b72f8fc..7e5a07a 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
@@ -413,6 +413,9 @@ public class TestKuduClient extends BaseKuduTest {
row.addString("key", String.format("key_%02d", i));
row.addString("c1", "c1_" + i);
row.addString("c2", "c2_" + i);
+ if (i % 2 == 0) {
+ row.addString("c3", "c3_" + i);
+ }
session.apply(insert);
}
session.flush();
@@ -444,8 +447,20 @@ public class TestKuduClient extends BaseKuduTest {
// IS NOT NULL
assertEquals(100, scanTableToStrings(table,
- KuduPredicate.isNotNull(schema.getColumn("c2")),
- KuduPredicate.isNotNull(schema.getColumn("key"))
+ KuduPredicate.newIsNotNullPredicate(schema.getColumn("c1")),
+ KuduPredicate.newIsNotNullPredicate(schema.getColumn("key"))
+ ).size());
+ assertEquals(50, scanTableToStrings(table,
+ KuduPredicate.newIsNotNullPredicate(schema.getColumn("c3"))
+ ).size());
+
+ // IS NULL
+ assertEquals(0, scanTableToStrings(table,
+ KuduPredicate.newIsNullPredicate(schema.getColumn("c2")),
+ KuduPredicate.newIsNullPredicate(schema.getColumn("key"))
+ ).size());
+ assertEquals(50, scanTableToStrings(table,
+ KuduPredicate.newIsNullPredicate(schema.getColumn("c3"))
).size());
// IN list
@@ -460,7 +475,7 @@ public class TestKuduClient extends BaseKuduTest {
assertEquals(2, scanTableToStrings(table,
KuduPredicate.newInListPredicate(schema.getColumn("c2"),
ImmutableList.of("c2_30", "c2_1", "invalid", "c2_99")),
- KuduPredicate.isNotNull(schema.getColumn("c2")),
+ KuduPredicate.newIsNotNullPredicate(schema.getColumn("c2")),
KuduPredicate.newInListPredicate(schema.getColumn("key"),
ImmutableList.of("key_30", "key_45", "invalid", "key_99"))
).size());
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
index 5afdd11..d192be8 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
@@ -58,7 +58,7 @@ public class TestKuduPredicate {
new ColumnSchema.ColumnSchemaBuilder("double", Type.DOUBLE).build();
private static final ColumnSchema stringCol =
- new ColumnSchema.ColumnSchemaBuilder("string", Type.STRING).build();
+ new ColumnSchema.ColumnSchemaBuilder("string", Type.STRING).nullable(true).build();
private static final ColumnSchema binaryCol =
new ColumnSchema.ColumnSchemaBuilder("binary", Type.BINARY).build();
@@ -95,6 +95,7 @@ public class TestKuduPredicate {
public void testMergeInt() {
// Equality + Equality
+ //--------------------
// |
// |
@@ -112,6 +113,7 @@ public class TestKuduPredicate {
KuduPredicate.none(intCol));
// Range + Equality
+ //--------------------
// [-------->
// |
@@ -146,6 +148,7 @@ public class TestKuduPredicate {
KuduPredicate.none(intCol));
// Unbounded Range + Unbounded Range
+ //--------------------
// [--------> AND
// [-------->
@@ -215,6 +218,7 @@ public class TestKuduPredicate {
KuduPredicate.none(intCol));
// Range + Range
+ //--------------------
// [--------) AND
// [--------)
@@ -265,6 +269,7 @@ public class TestKuduPredicate {
KuduPredicate.none(intCol));
// Lower Bound + Range
+ //--------------------
// [------------>
// [---)
@@ -299,6 +304,7 @@ public class TestKuduPredicate {
KuduPredicate.none(intCol));
// Upper Bound + Range
+ //--------------------
// <------------)
// [---)
@@ -326,6 +332,7 @@ public class TestKuduPredicate {
intRange(0, 5));
// Range + Equality
+ //--------------------
// [---) AND
// |
@@ -368,6 +375,7 @@ public class TestKuduPredicate {
KuduPredicate.none(intCol));
// IN list + IN list
+ //--------------------
// | | |
// | | |
@@ -382,15 +390,18 @@ public class TestKuduPredicate {
KuduPredicate.none(intCol));
// IN list + NOT NULL
+ //--------------------
+
testMerge(intInList(10),
- KuduPredicate.isNotNull(intCol),
+ KuduPredicate.newIsNotNullPredicate(intCol),
KuduPredicate.newComparisonPredicate(intCol, EQUAL, 10));
testMerge(intInList(10, -100),
- KuduPredicate.isNotNull(intCol),
+ KuduPredicate.newIsNotNullPredicate(intCol),
intInList(-100, 10));
// IN list + Equality
+ //--------------------
// | | |
// |
@@ -409,6 +420,7 @@ public class TestKuduPredicate {
KuduPredicate.none(intCol));
// IN list + Range
+ //--------------------
// | | | | |
// [---)
@@ -475,6 +487,7 @@ public class TestKuduPredicate {
KuduPredicate.none(intCol));
// None
+ //--------------------
// None AND
// [---->
@@ -483,6 +496,7 @@ public class TestKuduPredicate {
testMerge(KuduPredicate.none(intCol),
KuduPredicate.newComparisonPredicate(intCol, GREATER_EQUAL, 0),
KuduPredicate.none(intCol));
+
// None AND
// <----)
// =
@@ -514,6 +528,141 @@ public class TestKuduPredicate {
testMerge(KuduPredicate.none(intCol),
KuduPredicate.none(intCol),
KuduPredicate.none(intCol));
+
+ // IS NOT NULL
+ //--------------------
+
+ // IS NOT NULL AND
+ // NONE
+ // =
+ // NONE
+ testMerge(KuduPredicate.newIsNotNullPredicate(intCol),
+ KuduPredicate.none(intCol),
+ KuduPredicate.none(intCol));
+
+ // IS NOT NULL AND
+ // IS NULL
+ // =
+ // NONE
+ testMerge(KuduPredicate.newIsNotNullPredicate(intCol),
+ KuduPredicate.newIsNullPredicate(intCol),
+ KuduPredicate.none(intCol));
+
+ // IS NOT NULL AND
+ // IS NOT NULL
+ // =
+ // IS NOT NULL
+ testMerge(KuduPredicate.newIsNotNullPredicate(intCol),
+ KuduPredicate.newIsNotNullPredicate(intCol),
+ KuduPredicate.newIsNotNullPredicate(intCol));
+
+ // IS NOT NULL AND
+ // |
+ // =
+ // |
+ testMerge(KuduPredicate.newIsNotNullPredicate(intCol),
+ KuduPredicate.newComparisonPredicate(intCol, EQUAL, 5),
+ KuduPredicate.newComparisonPredicate(intCol, EQUAL, 5));
+
+ // IS NOT NULL AND
+ // [------->
+ // =
+ // [------->
+ testMerge(KuduPredicate.newIsNotNullPredicate(intCol),
+ KuduPredicate.newComparisonPredicate(intCol, GREATER_EQUAL, 5),
+ KuduPredicate.newComparisonPredicate(intCol, GREATER_EQUAL, 5));
+
+ // IS NOT NULL AND
+ // <---------)
+ // =
+ // <---------)
+ testMerge(KuduPredicate.newIsNotNullPredicate(intCol),
+ KuduPredicate.newComparisonPredicate(intCol, LESS, 5),
+ KuduPredicate.newComparisonPredicate(intCol, LESS, 5));
+
+ // IS NOT NULL AND
+ // [-------)
+ // =
+ // [-------)
+ testMerge(KuduPredicate.newIsNotNullPredicate(intCol),
+ intRange(0, 12),
+ intRange(0, 12));
+
+
+ // IS NOT NULL AND
+ // | | |
+ // =
+ // | | |
+ testMerge(KuduPredicate.newIsNotNullPredicate(intCol),
+ intInList(0, 10, 20),
+ intInList(0, 10, 20));
+
+ // IS NULL
+ //--------------------
+
+ // IS NULL AND
+ // NONE
+ // =
+ // NONE
+ testMerge(KuduPredicate.newIsNullPredicate(intCol),
+ KuduPredicate.none(intCol),
+ KuduPredicate.none(intCol));
+
+ // IS NULL AND
+ // IS NULL
+ // =
+ // IS_NULL
+ testMerge(KuduPredicate.newIsNullPredicate(intCol),
+ KuduPredicate.newIsNullPredicate(intCol),
+ KuduPredicate.newIsNullPredicate(intCol));
+
+ // IS NULL AND
+ // IS NOT NULL
+ // =
+ // NONE
+ testMerge(KuduPredicate.newIsNullPredicate(intCol),
+ KuduPredicate.newIsNotNullPredicate(intCol),
+ KuduPredicate.none(intCol));
+
+ // IS NULL AND
+ // |
+ // =
+ // NONE
+ testMerge(KuduPredicate.newIsNullPredicate(intCol),
+ KuduPredicate.newComparisonPredicate(intCol, EQUAL, 5),
+ KuduPredicate.none(intCol));
+
+ // IS NULL AND
+ // [------->
+ // =
+ // NONE
+ testMerge(KuduPredicate.newIsNullPredicate(intCol),
+ KuduPredicate.newComparisonPredicate(intCol, GREATER_EQUAL, 0),
+ KuduPredicate.none(intCol));
+
+ // IS NULL AND
+ // <---------)
+ // =
+ // NONE
+ testMerge(KuduPredicate.newIsNullPredicate(intCol),
+ KuduPredicate.newComparisonPredicate(intCol, LESS, 5),
+ KuduPredicate.none(intCol));
+
+ // IS NULL AND
+ // [-------)
+ // =
+ // NONE
+ testMerge(KuduPredicate.newIsNullPredicate(intCol),
+ intRange(0, 12),
+ KuduPredicate.none(intCol));
+
+ // IS NULL AND
+ // | | |
+ // =
+ // NONE
+ testMerge(KuduPredicate.newIsNullPredicate(intCol),
+ intInList(0, 10, 20),
+ KuduPredicate.none(intCol));
}
/**
@@ -558,7 +707,7 @@ public class TestKuduPredicate {
// [----->
// | | | |
// =
- // [--)
+ // | | |
testMerge(KuduPredicate.newComparisonPredicate(stringCol, GREATER_EQUAL, "a"),
stringInList("a", "c", "b", ""),
stringInList("a", "b", "c"));
@@ -566,8 +715,8 @@ public class TestKuduPredicate {
// IS NOT NULL
// | | | |
// =
- // [--)
- testMerge(KuduPredicate.isNotNull(stringCol),
+ // | | | |
+ testMerge(KuduPredicate.newIsNotNullPredicate(stringCol),
stringInList("a", "c", "b", ""),
stringInList("", "a", "b", "c"));
}
@@ -576,7 +725,7 @@ public class TestKuduPredicate {
public void testBoolean() {
// b >= false
- Assert.assertEquals(KuduPredicate.isNotNull(boolCol),
+ Assert.assertEquals(KuduPredicate.newIsNotNullPredicate(boolCol),
KuduPredicate.newComparisonPredicate(boolCol, GREATER_EQUAL, false));
// b > false
Assert.assertEquals(KuduPredicate.newComparisonPredicate(boolCol, EQUAL, true),
@@ -604,7 +753,7 @@ public class TestKuduPredicate {
Assert.assertEquals(KuduPredicate.newComparisonPredicate(boolCol, EQUAL, false),
KuduPredicate.newComparisonPredicate(boolCol, LESS, true));
// b <= true
- Assert.assertEquals(KuduPredicate.isNotNull(boolCol),
+ Assert.assertEquals(KuduPredicate.newIsNotNullPredicate(boolCol),
KuduPredicate.newComparisonPredicate(boolCol, LESS_EQUAL, true));
// b IN ()
@@ -619,7 +768,7 @@ public class TestKuduPredicate {
boolInList(false));
// b IN (false, true)
- Assert.assertEquals(KuduPredicate.isNotNull(boolCol),
+ Assert.assertEquals(KuduPredicate.newIsNotNullPredicate(boolCol),
boolInList(false, true, false, true));
}
@@ -638,7 +787,7 @@ public class TestKuduPredicate {
testMerge(KuduPredicate.newComparisonPredicate(boolCol, GREATER_EQUAL, false),
KuduPredicate.newComparisonPredicate(boolCol, LESS_EQUAL, true),
- KuduPredicate.isNotNull(boolCol));
+ KuduPredicate.newIsNotNullPredicate(boolCol));
testMerge(KuduPredicate.newComparisonPredicate(byteCol, GREATER_EQUAL, 0),
KuduPredicate.newComparisonPredicate(byteCol, LESS, 10),
@@ -728,21 +877,21 @@ public class TestKuduPredicate {
KuduPredicate.newComparisonPredicate(binaryCol, LESS, new byte[] { (byte) 10, (byte) 0 }));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(byteCol, LESS_EQUAL, Byte.MAX_VALUE),
- KuduPredicate.isNotNull(byteCol));
+ KuduPredicate.newIsNotNullPredicate(byteCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(shortCol, LESS_EQUAL, Short.MAX_VALUE),
- KuduPredicate.isNotNull(shortCol));
+ KuduPredicate.newIsNotNullPredicate(shortCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(intCol, LESS_EQUAL, Integer.MAX_VALUE),
- KuduPredicate.isNotNull(intCol));
+ KuduPredicate.newIsNotNullPredicate(intCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(longCol, LESS_EQUAL, Long.MAX_VALUE),
- KuduPredicate.isNotNull(longCol));
+ KuduPredicate.newIsNotNullPredicate(longCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(floatCol, LESS_EQUAL, Float.MAX_VALUE),
KuduPredicate.newComparisonPredicate(floatCol, LESS, Float.POSITIVE_INFINITY));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(floatCol, LESS_EQUAL, Float.POSITIVE_INFINITY),
- KuduPredicate.isNotNull(floatCol));
+ KuduPredicate.newIsNotNullPredicate(floatCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(doubleCol, LESS_EQUAL, Double.MAX_VALUE),
KuduPredicate.newComparisonPredicate(doubleCol, LESS, Double.POSITIVE_INFINITY));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(doubleCol, LESS_EQUAL, Double.POSITIVE_INFINITY),
- KuduPredicate.isNotNull(doubleCol));
+ KuduPredicate.newIsNotNullPredicate(doubleCol));
}
@Test
@@ -805,21 +954,21 @@ public class TestKuduPredicate {
@Test
public void testGreaterEqual() {
Assert.assertEquals(KuduPredicate.newComparisonPredicate(byteCol, GREATER_EQUAL, Byte.MIN_VALUE),
- KuduPredicate.isNotNull(byteCol));
+ KuduPredicate.newIsNotNullPredicate(byteCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(shortCol, GREATER_EQUAL, Short.MIN_VALUE),
- KuduPredicate.isNotNull(shortCol));
+ KuduPredicate.newIsNotNullPredicate(shortCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(intCol, GREATER_EQUAL, Integer.MIN_VALUE),
- KuduPredicate.isNotNull(intCol));
+ KuduPredicate.newIsNotNullPredicate(intCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(longCol, GREATER_EQUAL, Long.MIN_VALUE),
- KuduPredicate.isNotNull(longCol));
+ KuduPredicate.newIsNotNullPredicate(longCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(floatCol, GREATER_EQUAL, Float.NEGATIVE_INFINITY),
- KuduPredicate.isNotNull(floatCol));
+ KuduPredicate.newIsNotNullPredicate(floatCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(doubleCol, GREATER_EQUAL, Double.NEGATIVE_INFINITY),
- KuduPredicate.isNotNull(doubleCol));
+ KuduPredicate.newIsNotNullPredicate(doubleCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(stringCol, GREATER_EQUAL, ""),
- KuduPredicate.isNotNull(stringCol));
+ KuduPredicate.newIsNotNullPredicate(stringCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(binaryCol, GREATER_EQUAL, new byte[] {}),
- KuduPredicate.isNotNull(binaryCol));
+ KuduPredicate.newIsNotNullPredicate(binaryCol));
Assert.assertEquals(KuduPredicate.newComparisonPredicate(byteCol, GREATER_EQUAL, Byte.MAX_VALUE),
KuduPredicate.newComparisonPredicate(byteCol, EQUAL, Byte.MAX_VALUE));
@@ -858,7 +1007,12 @@ public class TestKuduPredicate {
Assert.assertEquals("`int` IN (-10, 0, 10)",
intInList(10, 0, -10).toString());
Assert.assertEquals("`string` IS NOT NULL",
- KuduPredicate.isNotNull(stringCol).toString());
+ KuduPredicate.newIsNotNullPredicate(stringCol).toString());
+ Assert.assertEquals("`string` IS NULL",
+ KuduPredicate.newIsNullPredicate(stringCol).toString());
+ // IS NULL predicate on non-nullable column = NONE predicate
+ Assert.assertEquals("`int` NONE",
+ KuduPredicate.newIsNullPredicate(intCol).toString());
Assert.assertEquals("`bool` = true", KuduPredicate.newInListPredicate(
boolCol, ImmutableList.of(true)).toString());
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
index 2408adb..7b9c51e 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
@@ -629,6 +629,10 @@ public class TestPartitionPruner extends BaseKuduTest {
// timestamp IS NOT NULL
assertEquals(4, countPartitions(table, partitions,
- KuduPredicate.isNotNull(timestamp)));
+ KuduPredicate.newIsNotNullPredicate(timestamp)));
+
+ // timestamp IS NULL
+ assertEquals(0, countPartitions(table, partitions,
+ KuduPredicate.newIsNullPredicate(timestamp)));
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanPredicate.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanPredicate.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanPredicate.java
index 3af4dfe..d89800c 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanPredicate.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestScanPredicate.java
@@ -212,6 +212,12 @@ public class TestScanPredicate extends BaseKuduTest {
KuduPredicate.newComparisonPredicate(col, ComparisonOp.LESS, v);
Assert.assertEquals(values.headSet(v).size(), countRows(table, less));
}
+
+ KuduPredicate isNotNull = KuduPredicate.newIsNotNullPredicate(col);
+ Assert.assertEquals(values.size(), countRows(table, isNotNull));
+
+ KuduPredicate isNull = KuduPredicate.newIsNullPredicate(col);
+ Assert.assertEquals(1, countRows(table, isNull));
}
@Test
@@ -446,6 +452,12 @@ public class TestScanPredicate extends BaseKuduTest {
KuduPredicate.newComparisonPredicate(col, ComparisonOp.LESS, v);
Assert.assertEquals(values.headSet(v).size(), countRows(table, less));
}
+
+ KuduPredicate isNotNull = KuduPredicate.newIsNotNullPredicate(col);
+ Assert.assertEquals(values.size(), countRows(table, isNotNull));
+
+ KuduPredicate isNull = KuduPredicate.newIsNullPredicate(col);
+ Assert.assertEquals(1, countRows(table, isNull));
}
@Test
@@ -499,6 +511,12 @@ public class TestScanPredicate extends BaseKuduTest {
KuduPredicate.newComparisonPredicate(col, ComparisonOp.LESS, v);
Assert.assertEquals(values.headSet(v).size(), countRows(table, less));
}
+
+ KuduPredicate isNotNull = KuduPredicate.newIsNotNullPredicate(col);
+ Assert.assertEquals(values.size(), countRows(table, isNotNull));
+
+ KuduPredicate isNull = KuduPredicate.newIsNullPredicate(col);
+ Assert.assertEquals(1, countRows(table, isNull));
}
@Test
@@ -552,6 +570,12 @@ public class TestScanPredicate extends BaseKuduTest {
KuduPredicate.newComparisonPredicate(col, ComparisonOp.LESS, v);
Assert.assertEquals(values.headSet(v).size(), countRows(table, less));
}
+
+ KuduPredicate isNotNull = KuduPredicate.newIsNotNullPredicate(col);
+ Assert.assertEquals(values.size(), countRows(table, isNotNull));
+
+ KuduPredicate isNull = KuduPredicate.newIsNullPredicate(col);
+ Assert.assertEquals(1, countRows(table, isNull));
}
@Test
@@ -606,5 +630,11 @@ public class TestScanPredicate extends BaseKuduTest {
KuduPredicate.newComparisonPredicate(col, ComparisonOp.LESS, v);
Assert.assertEquals(values.headSet(s).size(), countRows(table, less));
}
+
+ KuduPredicate isNotNull = KuduPredicate.newIsNotNullPredicate(col);
+ Assert.assertEquals(values.size(), countRows(table, isNotNull));
+
+ KuduPredicate isNull = KuduPredicate.newIsNullPredicate(col);
+ Assert.assertEquals(1, countRows(table, isNull));
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/cfile/binary_plain_block.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/binary_plain_block.cc b/src/kudu/cfile/binary_plain_block.cc
index ecc565f..a9eea75 100644
--- a/src/kudu/cfile/binary_plain_block.cc
+++ b/src/kudu/cfile/binary_plain_block.cc
@@ -300,6 +300,7 @@ Status BinaryPlainBlockDecoder::CopyNextValues(size_t* n, ColumnDataView* dst) {
CHECK(out_arena->RelocateSlice(elem, out));
});
}
+
Status BinaryPlainBlockDecoder::CopyNextAndEval(size_t* n,
ColumnMaterializationContext* ctx,
SelectionVectorView* sel,
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 95b8b97..e6e88a1 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -4119,6 +4119,199 @@ TEST_F(ClientTest, TestReadAtSnapshotNoTimestampSet) {
EXPECT_EQ(kTabletsNum * kRowsPerTablet, total_row_count);
}
+enum IntEncoding {
+ kPlain,
+ kBitShuffle,
+ kRunLength
+};
+
+class IntEncodingNullPredicatesTest : public ClientTest,
+ public ::testing::WithParamInterface<IntEncoding> {
+};
+
+TEST_P(IntEncodingNullPredicatesTest, TestIntEncodings) {
+ // Create table with appropriate encoded, nullable column.
+ KuduSchema schema;
+ KuduSchemaBuilder b;
+ b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
+ auto int_col = b.AddColumn("int_val")->Type(KuduColumnSchema::INT32);
+ IntEncoding enc = GetParam();
+ switch (enc) {
+ case kPlain:
+ int_col->Encoding(KuduColumnStorageAttributes::PLAIN_ENCODING);
+ break;
+ case kBitShuffle:
+ int_col->Encoding(KuduColumnStorageAttributes::BIT_SHUFFLE);
+ break;
+ case kRunLength:
+ int_col->Encoding(KuduColumnStorageAttributes::RLE);
+ break;
+ }
+ ASSERT_OK(b.Build(&schema));
+
+ string table_name = "IntEncodingNullPredicatesTestTable";
+ unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+ ASSERT_OK(table_creator->table_name(table_name)
+ .schema(&schema)
+ .num_replicas(1)
+ .set_range_partition_columns({ "key" })
+ .Create());
+
+ shared_ptr<KuduTable> table;
+ ASSERT_OK(client_->OpenTable(table_name, &table));
+
+ // Insert rows.
+ shared_ptr<KuduSession> session = table->client()->NewSession();
+ ASSERT_OK(session->SetFlushMode(KuduSession::MANUAL_FLUSH));
+ session->SetTimeoutMillis(5000);
+ const size_t kNumRows = AllowSlowTests() ? 10000 : 10000;
+ for (int i = 0; i < kNumRows; i++) {
+ KuduInsert* insert = table->NewInsert();
+ KuduPartialRow* row = insert->mutable_row();
+ ASSERT_OK(row->SetInt32("key", i));
+ if (i % 2 == 0) {
+ ASSERT_OK(row->SetInt32("int_val", i));
+ } else {
+ ASSERT_OK(row->SetNull("int_val"));
+ }
+ ASSERT_OK(session->Apply(insert));
+ if (i % 10001 == 0) {
+ ASSERT_OK(session->Flush());
+ }
+ }
+ ASSERT_OK(session->Flush());
+
+ // Scan rows and check for correct counts.
+ { // IS NULL
+ KuduScanner scanner(table.get());
+ KuduPredicate *p = table->NewIsNullPredicate("int_val");
+ ASSERT_OK(scanner.AddConjunctPredicate(p));
+ ASSERT_OK(scanner.Open());
+ int count = 0;
+ KuduScanBatch batch;
+ while (scanner.HasMoreRows()) {
+ CHECK_OK(scanner.NextBatch(&batch));
+ count += batch.NumRows();
+ }
+ ASSERT_EQ(kNumRows / 2, count);
+ }
+
+ { // IS NOT NULL
+ KuduScanner scanner(table.get());
+ KuduPredicate *p = table->NewIsNotNullPredicate("int_val");
+ ASSERT_OK(scanner.AddConjunctPredicate(p));
+ ASSERT_OK(scanner.Open());
+ int count = 0;
+ KuduScanBatch batch;
+ while (scanner.HasMoreRows()) {
+ CHECK_OK(scanner.NextBatch(&batch));
+ count += batch.NumRows();
+ }
+ ASSERT_EQ(kNumRows / 2, count);
+ }
+}
+
+INSTANTIATE_TEST_CASE_P(IntColEncodings,
+ IntEncodingNullPredicatesTest,
+ ::testing::Values(kPlain, kBitShuffle, kRunLength));
+
+
+enum BinaryEncoding {
+ kPlainBin,
+ kPrefix,
+ kDictionary
+};
+
+class BinaryEncodingNullPredicatesTest : public ClientTest,
+ public ::testing::WithParamInterface<IntEncoding> {
+};
+
+TEST_P(BinaryEncodingNullPredicatesTest, TestBinaryEncodings) {
+ // Create table with appropriate encoded, nullable column.
+ KuduSchema schema;
+ KuduSchemaBuilder b;
+ b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
+ auto string_col = b.AddColumn("string_val")->Type(KuduColumnSchema::STRING);
+ IntEncoding enc = GetParam();
+ switch (enc) {
+ case kPlainBin:
+ string_col->Encoding(KuduColumnStorageAttributes::PLAIN_ENCODING);
+ break;
+ case kPrefix:
+ string_col->Encoding(KuduColumnStorageAttributes::PREFIX_ENCODING);
+ break;
+ case kDictionary:
+ string_col->Encoding(KuduColumnStorageAttributes::DICT_ENCODING);
+ break;
+ }
+ ASSERT_OK(b.Build(&schema));
+
+ string table_name = "BinaryEncodingNullPredicatesTestTable";
+ unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+ ASSERT_OK(table_creator->table_name(table_name)
+ .schema(&schema)
+ .num_replicas(1)
+ .set_range_partition_columns({ "key" })
+ .Create());
+
+ shared_ptr<KuduTable> table;
+ ASSERT_OK(client_->OpenTable(table_name, &table));
+
+ // Insert rows.
+ shared_ptr<KuduSession> session = table->client()->NewSession();
+ ASSERT_OK(session->SetFlushMode(KuduSession::MANUAL_FLUSH));
+ session->SetTimeoutMillis(5000);
+ const size_t kNumRows = AllowSlowTests() ? 10000 : 1000;
+ for (int i = 0; i < kNumRows; i++) {
+ KuduInsert* insert = table->NewInsert();
+ KuduPartialRow* row = insert->mutable_row();
+ ASSERT_OK(row->SetInt32("key", i));
+ if (i % 2 == 0) {
+ ASSERT_OK(row->SetString("string_val", Substitute("taco %d", i % 25)));
+ } else {
+ ASSERT_OK(row->SetNull("string_val"));
+ }
+ ASSERT_OK(session->Apply(insert));
+ if (i % 10001 == 0) {
+ ASSERT_OK(session->Flush());
+ }
+ }
+ ASSERT_OK(session->Flush());
+
+ // Scan rows and check for correct counts.
+ { // IS NULL
+ KuduScanner scanner(table.get());
+ KuduPredicate *p = table->NewIsNullPredicate("string_val");
+ ASSERT_OK(scanner.AddConjunctPredicate(p));
+ ASSERT_OK(scanner.Open());
+ int count = 0;
+ KuduScanBatch batch;
+ while (scanner.HasMoreRows()) {
+ CHECK_OK(scanner.NextBatch(&batch));
+ count += batch.NumRows();
+ }
+ ASSERT_EQ(kNumRows / 2, count);
+ }
+
+ { // IS NOT NULL
+ KuduScanner scanner(table.get());
+ KuduPredicate *p = table->NewIsNotNullPredicate("string_val");
+ ASSERT_OK(scanner.AddConjunctPredicate(p));
+ ASSERT_OK(scanner.Open());
+ int count = 0;
+ KuduScanBatch batch;
+ while (scanner.HasMoreRows()) {
+ CHECK_OK(scanner.NextBatch(&batch));
+ count += batch.NumRows();
+ }
+ ASSERT_EQ(kNumRows / 2, count);
+ }
+}
+
+INSTANTIATE_TEST_CASE_P(BinaryColEncodings,
+ BinaryEncodingNullPredicatesTest,
+ ::testing::Values(kPlainBin, kPrefix, kDictionary));
+
TEST_F(ClientTest, TestClonePredicates) {
ASSERT_NO_FATAL_FAILURE(InsertTestRows(client_table_.get(),
2, 0));
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index c6ef71f..d060852 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -64,6 +64,7 @@
#include "kudu/util/logging.h"
#include "kudu/util/net/dns_resolver.h"
#include "kudu/util/oid_generator.h"
+#include "kudu/util/scoped_cleanup.h"
#include "kudu/util/version_info.h"
using kudu::master::AlterTableRequestPB;
@@ -712,38 +713,44 @@ const PartitionSchema& KuduTable::partition_schema() const {
KuduPredicate* KuduTable::NewComparisonPredicate(const Slice& col_name,
KuduPredicate::ComparisonOp op,
KuduValue* value) {
- StringPiece name_sp(reinterpret_cast<const char*>(col_name.data()), col_name.size());
- const Schema* s = data_->schema_.schema_;
- int col_idx = s->find_column(name_sp);
- if (col_idx == Schema::kColumnNotFound) {
- // Since this function doesn't return an error, instead we create a special
- // predicate that just returns the errors when we add it to the scanner.
- //
- // This makes the API more "fluent".
- delete value; // we always take ownership of 'value'.
- return new KuduPredicate(new ErrorPredicateData(
- Status::NotFound("column not found", col_name)));
- }
-
- return new KuduPredicate(new ComparisonPredicateData(s->column(col_idx), op, value));
+ // We always take ownership of value; this ensures cleanup if the predicate is invalid.
+ auto cleanup = MakeScopedCleanup([&]() {
+ delete value;
+ });
+ const Schema& schema = *data_->schema_.schema_;
+ return data_->MakePredicate(col_name, schema, [&](const ColumnSchema& col_schema) {
+ // Ownership of value is passed to the valid returned predicate.
+ cleanup.cancel();
+ return new KuduPredicate(new ComparisonPredicateData(col_schema, op, value));
+ });
}
KuduPredicate* KuduTable::NewInListPredicate(const Slice& col_name,
vector<KuduValue*>* values) {
- StringPiece name_sp(reinterpret_cast<const char*>(col_name.data()), col_name.size());
- const Schema* s = data_->schema_.schema_;
- int col_idx = s->find_column(name_sp);
- if (col_idx == Schema::kColumnNotFound) {
- // Since this function doesn't return an error, instead we create a special
- // predicate that just returns the errors when we add it to the scanner.
- //
- // This makes the API more "fluent".
- STLDeleteElements(values); // we always take ownership of 'values'.
- delete values;
- return new KuduPredicate(new ErrorPredicateData(
- Status::NotFound("column not found", col_name)));
- }
- return new KuduPredicate(new InListPredicateData(s->column(col_idx), values));
+ // We always take ownership of values; this ensures cleanup if the predicate is invalid.
+ auto cleanup = MakeScopedCleanup([&]() {
+ STLDeleteElements(values);
+ });
+ const Schema& schema = *data_->schema_.schema_;
+ return data_->MakePredicate(col_name, schema, [&](const ColumnSchema& col_schema) {
+ // Ownership of values is passed to the valid returned predicate.
+ cleanup.cancel();
+ return new KuduPredicate(new InListPredicateData(col_schema, values));
+ });
+}
+
+KuduPredicate* KuduTable::NewIsNotNullPredicate(const Slice& col_name) {
+ const Schema& schema = *data_->schema_.schema_;
+ return data_->MakePredicate(col_name, schema, [&](const ColumnSchema& col_schema) {
+ return new KuduPredicate(new IsNotNullPredicateData(col_schema));
+ });
+}
+
+KuduPredicate* KuduTable::NewIsNullPredicate(const Slice& col_name) {
+ const Schema& schema = *data_->schema_.schema_;
+ return data_->MakePredicate(col_name, schema, [&](const ColumnSchema& col_schema) {
+ return new KuduPredicate(new IsNullPredicateData(col_schema));
+ });
}
////////////////////////////////////////////////////////////
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/client/client.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index 3255b0c..16cea6a 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -894,6 +894,30 @@ class KUDU_EXPORT KuduTable : public sp::enable_shared_from_this<KuduTable> {
KuduPredicate* NewInListPredicate(const Slice& col_name,
std::vector<KuduValue*>* values);
+ /// Create a new IS NOT NULL predicate which can be used for scanners on this
+ /// table.
+ ///
+ /// @param [in] col_name
+ /// Name of the column to which the predicate applies
+ /// @return Raw pointer to an IS NOT NULL predicate. The caller owns the
+ /// predicate until it is passed into KuduScanner::AddConjunctPredicate().
+ /// In the case of an error (e.g. an invalid column name), a non-NULL
+ /// value is still returned. The error will be returned when attempting
+ /// to add this predicate to a KuduScanner.
+ KuduPredicate* NewIsNotNullPredicate(const Slice& col_name);
+
+ /// Create a new IS NULL predicate which can be used for scanners on this
+ /// table.
+ ///
+ /// @param [in] col_name
+ /// Name of the column to which the predicate applies
+ /// @return Raw pointer to an IS NULL predicate. The caller owns the
+ /// predicate until it is passed into KuduScanner::AddConjunctPredicate().
+ /// In the case of an error (e.g. an invalid column name), a non-NULL
+ /// value is still returned. The error will be returned when attempting
+ /// to add this predicate to a KuduScanner.
+ KuduPredicate* NewIsNullPredicate(const Slice& col_name);
+
/// @return The KuduClient object associated with the table. The caller
/// should not free the returned pointer.
KuduClient* client() const;
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/client/predicate-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/predicate-test.cc b/src/kudu/client/predicate-test.cc
index 19c408d..76bf4c4 100644
--- a/src/kudu/client/predicate-test.cc
+++ b/src/kudu/client/predicate-test.cc
@@ -165,7 +165,7 @@ class PredicateTest : public KuduTest {
// Returns a vector of floating point numbers from -50.50 (inclusive) to 49.49
// (exclusive) (100 values), plus min, max, two normals around 0, two
- // subnormals around 0, positive and negatic infinity, and NaN.
+ // subnormals around 0, positive and negative infinity, and NaN.
template <typename T>
vector<T> CreateFloatingPointValues() {
vector<T> values;
@@ -231,9 +231,9 @@ class PredicateTest : public KuduTest {
}
// Check integer predicates against the specified table. The table must have
- // key/value rows with values from CreateIntValues, plus a null value.
+ // key/value rows with values from CreateIntValues, plus one null value.
template <typename T>
- void CheckIntPredicates(shared_ptr<KuduTable> table) {
+ void CheckIntPredicates(const shared_ptr<KuduTable>& table) {
vector<T> values = CreateIntValues<T>();
vector<T> test_values = CreateIntTestValues<T>();
ASSERT_EQ(values.size() + 1, CountRows(table, {}));
@@ -333,6 +333,13 @@ class PredicateTest : public KuduTest {
int count = CountMatchedRows<T>(values, vector<T>(test_values.begin(), end));
ASSERT_EQ(count, CountRows(table, { table->NewInListPredicate("value", &vals) }));
}
+
+ // IS NOT NULL predicate
+ ASSERT_EQ(CountRows(table, {}) - 1,
+ CountRows(table, { table->NewIsNotNullPredicate("value") }));
+
+ // IS NULL predicate
+ ASSERT_EQ(1, CountRows(table, { table->NewIsNullPredicate("value") }));
}
// Check string predicates against the specified table.
@@ -442,6 +449,13 @@ class PredicateTest : public KuduTest {
int count = CountMatchedRows<string>(values, vector<string>(test_values.begin(), end));
ASSERT_EQ(count, CountRows(table, { table->NewInListPredicate("value", &vals) }));
}
+
+ // IS NOT NULL predicate
+ ASSERT_EQ(CountRows(table, {}) - 1,
+ CountRows(table, { table->NewIsNotNullPredicate("value") }));
+
+ // IS NULL predicate
+ ASSERT_EQ(1, CountRows(table, { table->NewIsNullPredicate("value") }));
}
shared_ptr<KuduClient> client_;
@@ -561,6 +575,13 @@ TEST_F(PredicateTest, TestBoolPredicates) {
KuduPredicate* pred = table->NewInListPredicate("value", &values);
ASSERT_EQ(2, CountRows(table, { pred }));
}
+
+ // IS NOT NULL predicate
+ ASSERT_EQ(CountRows(table, {}) - 1,
+ CountRows(table, { table->NewIsNotNullPredicate("value") }));
+
+ // IS NULL predicate
+ ASSERT_EQ(1, CountRows(table, { table->NewIsNullPredicate("value") }));
}
TEST_F(PredicateTest, TestInt8Predicates) {
@@ -778,6 +799,13 @@ TEST_F(PredicateTest, TestFloatPredicates) {
int count = CountMatchedRows<float>(values, vector<float>(test_values.begin(), end));
ASSERT_EQ(count, CountRows(table, { table->NewInListPredicate("value", &vals) }));
}
+
+ // IS NOT NULL predicate
+ ASSERT_EQ(values.size(),
+ CountRows(table, { table->NewIsNotNullPredicate("value") }));
+
+ // IS NULL predicate
+ ASSERT_EQ(1, CountRows(table, { table->NewIsNullPredicate("value") }));
}
TEST_F(PredicateTest, TestDoublePredicates) {
@@ -895,6 +923,13 @@ TEST_F(PredicateTest, TestDoublePredicates) {
int count = CountMatchedRows<double>(values, vector<double>(test_values.begin(), end));
ASSERT_EQ(count, CountRows(table, { table->NewInListPredicate("value", &vals) }));
}
+
+ // IS NOT NULL predicate
+ ASSERT_EQ(values.size(),
+ CountRows(table, { table->NewIsNotNullPredicate("value") }));
+
+ // IS NULL predicate
+ ASSERT_EQ(1, CountRows(table, { table->NewIsNullPredicate("value") }));
}
TEST_F(PredicateTest, TestStringPredicates) {
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/client/scan_predicate-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_predicate-internal.h b/src/kudu/client/scan_predicate-internal.h
index e0757ae..c3716cc 100644
--- a/src/kudu/client/scan_predicate-internal.h
+++ b/src/kudu/client/scan_predicate-internal.h
@@ -116,6 +116,50 @@ class InListPredicateData : public KuduPredicate::Data {
std::vector<KuduValue*> vals_;
};
+// A predicate for selecting non-null values.
+class IsNotNullPredicateData : public KuduPredicate::Data {
+ public:
+ explicit IsNotNullPredicateData(ColumnSchema col)
+ : col_(std::move(col)) {
+ }
+
+ Status AddToScanSpec(ScanSpec* spec, Arena* /*arena*/) override {
+ spec->AddPredicate(ColumnPredicate::IsNotNull(col_));
+ return Status::OK();
+ }
+
+ IsNotNullPredicateData* Clone() const override {
+ return new IsNotNullPredicateData(col_);
+ }
+
+ private:
+ friend class KuduScanner;
+
+ ColumnSchema col_;
+};
+
+// A predicate for selecting null values.
+class IsNullPredicateData : public KuduPredicate::Data {
+ public:
+ explicit IsNullPredicateData(ColumnSchema col)
+ : col_(std::move(col)) {
+ }
+
+ Status AddToScanSpec(ScanSpec* spec, Arena* /*arena*/) override {
+ spec->AddPredicate(ColumnPredicate::IsNull(col_));
+ return Status::OK();
+ }
+
+ IsNullPredicateData* Clone() const override {
+ return new IsNullPredicateData(col_);
+ }
+
+ private:
+ friend class KuduScanner;
+
+ ColumnSchema col_;
+};
+
} // namespace client
} // namespace kudu
#endif /* KUDU_CLIENT_SCAN_PREDICATE_INTERNAL_H */
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/client/scan_predicate.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_predicate.h b/src/kudu/client/scan_predicate.h
index bb97151..16fd118 100644
--- a/src/kudu/client/scan_predicate.h
+++ b/src/kudu/client/scan_predicate.h
@@ -58,6 +58,8 @@ class KUDU_EXPORT KuduPredicate {
friend class ComparisonPredicateData;
friend class ErrorPredicateData;
friend class InListPredicateData;
+ friend class IsNotNullPredicateData;
+ friend class IsNullPredicateData;
friend class KuduTable;
friend class ScanConfiguration;
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/client/scan_token-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_token-internal.cc b/src/kudu/client/scan_token-internal.cc
index 678b915..31b22f5 100644
--- a/src/kudu/client/scan_token-internal.cc
+++ b/src/kudu/client/scan_token-internal.cc
@@ -170,6 +170,10 @@ Status KuduScanTokenBuilder::Data::Build(vector<KuduScanToken*>* tokens) {
KuduClient* client = table->client();
configuration_.OptimizeScanSpec();
+ if (configuration_.spec().CanShortCircuit()) {
+ return Status::OK();
+ }
+
ScanTokenPB pb;
pb.set_table_name(table->name());
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/client/scan_token-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_token-test.cc b/src/kudu/client/scan_token-test.cc
index 8875315..42028e3 100644
--- a/src/kudu/client/scan_token-test.cc
+++ b/src/kudu/client/scan_token-test.cc
@@ -195,6 +195,32 @@ TEST_F(ScanTokenTest, TestScanTokens) {
NO_FATALS(VerifyTabletInfo(tokens));
}
+ { // IS NOT NULL predicate
+ vector<KuduScanToken*> tokens;
+ ElementDeleter deleter(&tokens);
+ KuduScanTokenBuilder builder(table.get());
+ unique_ptr<KuduPredicate> predicate(table->NewIsNotNullPredicate("col"));
+ ASSERT_OK(builder.AddConjunctPredicate(predicate.release()));
+ ASSERT_OK(builder.Build(&tokens));
+
+ ASSERT_GE(8, tokens.size());
+ ASSERT_EQ(200, CountRows(tokens));
+ NO_FATALS(VerifyTabletInfo(tokens));
+ }
+
+ { // IS NULL predicate
+ vector<KuduScanToken*> tokens;
+ ElementDeleter deleter(&tokens);
+ KuduScanTokenBuilder builder(table.get());
+ unique_ptr<KuduPredicate> predicate(table->NewIsNullPredicate("col"));
+ ASSERT_OK(builder.AddConjunctPredicate(predicate.release()));
+ ASSERT_OK(builder.Build(&tokens));
+
+ ASSERT_GE(0, tokens.size());
+ ASSERT_EQ(0, CountRows(tokens));
+ NO_FATALS(VerifyTabletInfo(tokens));
+ }
+
{ // primary key bound
vector<KuduScanToken*> tokens;
ElementDeleter deleter(&tokens);
@@ -307,6 +333,32 @@ TEST_F(ScanTokenTest, TestScanTokensWithNonCoveringRange) {
NO_FATALS(VerifyTabletInfo(tokens));
}
+ { // IS NOT NULL predicate
+ vector<KuduScanToken*> tokens;
+ ElementDeleter deleter(&tokens);
+ KuduScanTokenBuilder builder(table.get());
+ unique_ptr<KuduPredicate> predicate(table->NewIsNotNullPredicate("col"));
+ ASSERT_OK(builder.AddConjunctPredicate(predicate.release()));
+ ASSERT_OK(builder.Build(&tokens));
+
+ ASSERT_EQ(6, tokens.size());
+ ASSERT_EQ(300, CountRows(tokens));
+ NO_FATALS(VerifyTabletInfo(tokens));
+ }
+
+ { // IS NULL predicate
+ vector<KuduScanToken*> tokens;
+ ElementDeleter deleter(&tokens);
+ KuduScanTokenBuilder builder(table.get());
+ unique_ptr<KuduPredicate> predicate(table->NewIsNullPredicate("col"));
+ ASSERT_OK(builder.AddConjunctPredicate(predicate.release()));
+ ASSERT_OK(builder.Build(&tokens));
+
+ ASSERT_GE(0, tokens.size());
+ ASSERT_EQ(0, CountRows(tokens));
+ NO_FATALS(VerifyTabletInfo(tokens));
+ }
+
{ // primary key bound
vector<KuduScanToken*> tokens;
ElementDeleter deleter(&tokens);
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/client/table-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/table-internal.h b/src/kudu/client/table-internal.h
index a29c146..748a220 100644
--- a/src/kudu/client/table-internal.h
+++ b/src/kudu/client/table-internal.h
@@ -21,6 +21,7 @@
#include "kudu/common/partition.h"
#include "kudu/client/client.h"
+#include "kudu/client/scan_predicate-internal.h"
namespace kudu {
@@ -36,6 +37,23 @@ class KuduTable::Data {
PartitionSchema partition_schema);
~Data();
+ template<class PredicateMakerFunc>
+ static KuduPredicate* MakePredicate(const Slice& col_name,
+ const Schema& schema,
+ const PredicateMakerFunc& f) {
+ StringPiece name_sp(reinterpret_cast<const char*>(col_name.data()), col_name.size());
+ int col_idx = schema.find_column(name_sp);
+ if (col_idx == Schema::kColumnNotFound) {
+ // Since the new predicate functions don't return errors, instead we create a special
+ // predicate that just returns the errors when we add it to the scanner.
+ //
+ // This allows the predicate API to be more "fluent".
+ return new KuduPredicate(new ErrorPredicateData(
+ Status::NotFound("column not found", col_name)));
+ }
+ return f(schema.column(col_idx));
+ }
+
sp::shared_ptr<KuduClient> client_;
const std::string name_;
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/common/column_predicate-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/column_predicate-test.cc b/src/kudu/common/column_predicate-test.cc
index 9f56186..d8a4ada 100644
--- a/src/kudu/common/column_predicate-test.cc
+++ b/src/kudu/common/column_predicate-test.cc
@@ -660,6 +660,81 @@ class TestColumnPredicate : public KuduTest {
ColumnPredicate::InList(column, &bot_list),
ColumnPredicate::InList(column, &res_list),
PredicateType::InList);
+
+ // IS NULL
+
+ // IS NULL AND
+ // None
+ // =
+ // None
+ TestMerge(ColumnPredicate::IsNull(column),
+ ColumnPredicate::None(column),
+ ColumnPredicate::None(column),
+ PredicateType::None);
+
+ // IS NULL AND
+ // |
+ // =
+ // None
+ TestMerge(ColumnPredicate::IsNull(column),
+ ColumnPredicate::Equality(column, &values[0]),
+ ColumnPredicate::None(column),
+ PredicateType::None);
+
+ // IS NULL AND
+ // [-------)
+ // =
+ // None
+ TestMerge(ColumnPredicate::IsNull(column),
+ ColumnPredicate::Range(column, &values[0], &values[2]),
+ ColumnPredicate::None(column),
+ PredicateType::None);
+
+ // IS NULL AND
+ // [------->
+ // =
+ // None
+ TestMerge(ColumnPredicate::IsNull(column),
+ ColumnPredicate::Range(column, &values[0], nullptr),
+ ColumnPredicate::None(column),
+ PredicateType::None);
+
+ // IS NULL AND
+ // <-------)
+ // =
+ // None
+ TestMerge(ColumnPredicate::IsNull(column),
+ ColumnPredicate::Range(column, nullptr, &values[2]),
+ ColumnPredicate::None(column),
+ PredicateType::None);
+
+ // IS NULL AND
+ // | | |
+ // =
+ // None
+ bot_list = { &values[1], &values[3], &values[6] };
+ TestMerge(ColumnPredicate::IsNull(column),
+ ColumnPredicate::InList(column, &bot_list),
+ ColumnPredicate::None(column),
+ PredicateType::None);
+
+ // IS NULL AND
+ // IS NOT NULL
+ // =
+ // None
+ TestMerge(ColumnPredicate::IsNull(column),
+ ColumnPredicate::IsNotNull(column),
+ ColumnPredicate::None(column),
+ PredicateType::None);
+
+ // IS NULL AND
+ // IS NULL
+ // =
+ // IS NULL
+ TestMerge(ColumnPredicate::IsNull(column),
+ ColumnPredicate::IsNull(column),
+ ColumnPredicate::IsNull(column),
+ PredicateType::IsNull);
}
};
@@ -979,13 +1054,16 @@ TEST_F(TestColumnPredicate, TestSelectivity) {
ColumnSchema column_s("d", STRING, true);
// Predicate type
+ ASSERT_LT(SelectivityComparator(ColumnPredicate::IsNull(column_i32),
+ ColumnPredicate::Equality(column_i32, &one_32)),
+ 0);
ASSERT_LT(SelectivityComparator(ColumnPredicate::Equality(column_i32, &one_32),
ColumnPredicate::Range(column_d, &one_d, nullptr)),
0);
ASSERT_LT(SelectivityComparator(ColumnPredicate::Equality(column_i32, &one_32),
ColumnPredicate::IsNotNull(column_s)),
0);
- ASSERT_LT(SelectivityComparator(ColumnPredicate::Range(column_i64, &one_64, nullptr),
+ ASSERT_LT(SelectivityComparator(ColumnPredicate::Range(column_i32, &one_32, nullptr),
ColumnPredicate::IsNotNull(column_i32)),
0);
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/common/column_predicate.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/column_predicate.cc b/src/kudu/common/column_predicate.cc
index 0e8c959..3b80104 100644
--- a/src/kudu/common/column_predicate.cc
+++ b/src/kudu/common/column_predicate.cc
@@ -143,6 +143,12 @@ ColumnPredicate ColumnPredicate::IsNotNull(ColumnSchema column) {
return ColumnPredicate(PredicateType::IsNotNull, move(column), nullptr, nullptr);
}
+ColumnPredicate ColumnPredicate::IsNull(ColumnSchema column) {
+ return column.is_nullable() ?
+ ColumnPredicate(PredicateType::IsNull, move(column), nullptr, nullptr) :
+ None(move(column));
+}
+
ColumnPredicate ColumnPredicate::None(ColumnSchema column) {
return ColumnPredicate(PredicateType::None, move(column), nullptr, nullptr);
}
@@ -159,6 +165,7 @@ void ColumnPredicate::Simplify() {
case PredicateType::None:
case PredicateType::Equality:
case PredicateType::IsNotNull: return;
+ case PredicateType::IsNull: return;
case PredicateType::Range: {
DCHECK(lower_ != nullptr || upper_ != nullptr);
if (lower_ != nullptr && upper_ != nullptr) {
@@ -229,18 +236,13 @@ void ColumnPredicate::Merge(const ColumnPredicate& other) {
return;
};
case PredicateType::IsNotNull: {
- // NOT NULL is less selective than all other predicate types, so the
- // intersection of NOT NULL with any other predicate is just the other
- // predicate.
- //
- // Note: this will no longer be true when an IS NULL predicate type is
- // added.
- predicate_type_ = other.predicate_type_;
- lower_ = other.lower_;
- upper_ = other.upper_;
- values_ = other.values_;
+ MergeIntoIsNotNull(other);
return;
};
+ case PredicateType::IsNull: {
+ MergeIntoIsNull(other);
+ return;
+ }
case PredicateType::InList: {
MergeIntoInList(other);
return;
@@ -288,6 +290,10 @@ void ColumnPredicate::MergeIntoRange(const ColumnPredicate& other) {
return;
};
case PredicateType::IsNotNull: return;
+ case PredicateType::IsNull: {
+ SetToNone();
+ return;
+ }
case PredicateType::InList : {
// The InList predicate values are examined to check whether
// they lie in the range.
@@ -333,6 +339,10 @@ void ColumnPredicate::MergeIntoEquality(const ColumnPredicate& other) {
return;
};
case PredicateType::IsNotNull: return;
+ case PredicateType::IsNull: {
+ SetToNone();
+ return;
+ }
case PredicateType::InList : {
// The equality value needs to be a member of the InList
if (!other.CheckValueInList(lower_)) {
@@ -344,12 +354,47 @@ void ColumnPredicate::MergeIntoEquality(const ColumnPredicate& other) {
LOG(FATAL) << "unknown predicate type";
}
+void ColumnPredicate::MergeIntoIsNotNull(const ColumnPredicate &other) {
+ CHECK(predicate_type_ == PredicateType::IsNotNull);
+ switch (other.predicate_type()) {
+ // The intersection of NULL and IS NOT NULL is None.
+ case PredicateType::IsNull: {
+ SetToNone();
+ return;
+ }
+ default: {
+ // Otherwise, the intersection is the other predicate.
+ predicate_type_ = other.predicate_type_;
+ lower_ = other.lower_;
+ upper_ = other.upper_;
+ values_ = other.values_;
+ return;
+ }
+ }
+}
+
+void ColumnPredicate::MergeIntoIsNull(const ColumnPredicate &other) {
+ CHECK(predicate_type_ == PredicateType::IsNull);
+ switch (other.predicate_type()) {
+ // The intersection of IS NULL and IS NULL is IS NULL.
+ case PredicateType::IsNull: {
+ return;
+ }
+ default: {
+ // Otherwise, the intersection is None.
+ // NB: This will not be true if NULL is allowed in an InList predicate.
+ SetToNone();
+ return;
+ }
+ }
+}
+
void ColumnPredicate::MergeIntoInList(const ColumnPredicate &other) {
CHECK(predicate_type_ == PredicateType::InList);
DCHECK(values_.size() > 1);
switch (other.predicate_type()) {
- case PredicateType::None : {
+ case PredicateType::None: {
SetToNone();
return;
};
@@ -388,6 +433,10 @@ void ColumnPredicate::MergeIntoInList(const ColumnPredicate &other) {
return;
}
case PredicateType::IsNotNull: return;
+ case PredicateType::IsNull: {
+ SetToNone();
+ return;
+ }
case PredicateType::InList: {
// Merge the 'other' IN list into this IN list. The basic idea is to loop
// through this predicate list, retaining only the values which are also
@@ -505,6 +554,20 @@ void ColumnPredicate::EvaluateForPhysicalType(const ColumnBlock& block,
}
return;
};
+ case PredicateType::IsNull: {
+ if (!block.is_nullable()) {
+ BitmapChangeBits(sel->mutable_bitmap(), 0, block.nrows(), false);
+ return;
+ }
+ // TODO(wdberkeley): make this more efficient by using bitwise operations on the
+ // null and selection vectors.
+ for (size_t i = 0; i < block.nrows(); i++) {
+ if (sel->IsRowSelected(i) && !block.is_null(i)) {
+ BitmapClear(sel->mutable_bitmap(), i);
+ }
+ }
+ return;
+ }
case PredicateType::InList: {
ApplyPredicate(block, sel, [this] (const void* cell) {
return std::binary_search(values_.begin(), values_.end(), cell,
@@ -559,6 +622,9 @@ string ColumnPredicate::ToString() const {
case PredicateType::IsNotNull: {
return strings::Substitute("`$0` IS NOT NULL", column_.name());
};
+ case PredicateType::IsNull: {
+ return strings::Substitute("`$0` IS NULL", column_.name());
+ };
case PredicateType::InList: {
string ss = "`";
ss.append(column_.name());
@@ -602,7 +668,8 @@ bool ColumnPredicate::operator==(const ColumnPredicate& other) const {
return true;
};
case PredicateType::None:
- case PredicateType::IsNotNull: return true;
+ case PredicateType::IsNotNull:
+ case PredicateType::IsNull: return true;
}
LOG(FATAL) << "unknown predicate type";
}
@@ -625,10 +692,11 @@ int SelectivityRank(const ColumnPredicate& predicate) {
int rank;
switch (predicate.predicate_type()) {
case PredicateType::None: rank = 0; break;
- case PredicateType::Equality: rank = 1; break;
- case PredicateType::InList: rank = 2; break;
- case PredicateType::Range: rank = 3; break;
- case PredicateType::IsNotNull: rank = 4; break;
+ case PredicateType::IsNull: rank = 1; break;
+ case PredicateType::Equality: rank = 2; break;
+ case PredicateType::InList: rank = 3; break;
+ case PredicateType::Range: rank = 4; break;
+ case PredicateType::IsNotNull: rank = 5; break;
default: LOG(FATAL) << "unknown predicate type";
}
return rank * (kLargestTypeSize + 1) + predicate.column().type_info()->size();
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/common/column_predicate.h
----------------------------------------------------------------------
diff --git a/src/kudu/common/column_predicate.h b/src/kudu/common/column_predicate.h
index aa996a1..d28f9b4 100644
--- a/src/kudu/common/column_predicate.h
+++ b/src/kudu/common/column_predicate.h
@@ -46,6 +46,9 @@ enum class PredicateType {
// A predicate which evaluates to true if the value is not null.
IsNotNull,
+ // A predicate which evaluates to true if the value is null.
+ IsNull,
+
// A predicate which evaluates to true if the column value is present in
// a value list.
InList,
@@ -112,12 +115,20 @@ class ColumnPredicate {
// Creates a new IS NOT NULL predicate for the column.
static ColumnPredicate IsNotNull(ColumnSchema column);
+ // Creates a new IS NULL predicate for the column.
+ //
+ // If the column is non-nullable, returns a None predicate instead.
+ static ColumnPredicate IsNull(ColumnSchema column);
+
// Create a new IN <LIST> predicate for the column.
//
// The values are not copied, and must outlive the returned predicate.
// The InList will be simplified into an Equality, Range or None if possible.
static ColumnPredicate InList(ColumnSchema column, std::vector<const void*>* values);
+ // Creates a new predicate which matches no values.
+ static ColumnPredicate None(ColumnSchema column);
+
// Returns the type of this predicate.
PredicateType predicate_type() const {
return predicate_type_;
@@ -172,6 +183,9 @@ class ColumnPredicate {
case PredicateType::IsNotNull: {
return true;
};
+ case PredicateType::IsNull: {
+ return false;
+ };
case PredicateType::InList: {
return std::binary_search(values_.begin(), values_.end(), cell,
[] (const void* lhs, const void* rhs) {
@@ -227,9 +241,6 @@ class ColumnPredicate {
ColumnSchema column,
std::vector<const void*>* values);
- // Creates a new predicate which matches no values.
- static ColumnPredicate None(ColumnSchema column);
-
// Transition to a None predicate type.
void SetToNone();
@@ -242,6 +253,12 @@ class ColumnPredicate {
// Merge another predicate into this Equality predicate.
void MergeIntoEquality(const ColumnPredicate& other);
+ // Merge another predicate into this IS NOT NULL predicate.
+ void MergeIntoIsNotNull(const ColumnPredicate& other);
+
+ // Merge another predicate into this IS NULL predicate.
+ void MergeIntoIsNull(const ColumnPredicate& other);
+
// Templated evaluation to inline the dispatch of comparator. Templating this
// allows dispatch to occur only once per batch.
template <DataType PhysicalType>
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/common/common.proto
----------------------------------------------------------------------
diff --git a/src/kudu/common/common.proto b/src/kudu/common/common.proto
index 5f84175..ff501f1 100644
--- a/src/kudu/common/common.proto
+++ b/src/kudu/common/common.proto
@@ -314,10 +314,13 @@ message ColumnPredicatePB {
message IsNotNull {}
+ message IsNull {}
+
oneof predicate {
Range range = 2;
Equality equality = 3;
IsNotNull is_not_null = 4;
InList in_list = 5;
+ IsNull is_null = 6;
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/common/key_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/key_util.cc b/src/kudu/common/key_util.cc
index 451cadd..11a5744 100644
--- a/src/kudu/common/key_util.cc
+++ b/src/kudu/common/key_util.cc
@@ -232,7 +232,8 @@ int PushUpperBoundKeyPredicates(ColIdxIter first,
break_loop = true;
}
break;
- case PredicateType::IsNotNull:
+ case PredicateType::IsNotNull: // Fallthrough intended
+ case PredicateType::IsNull:
break_loop = true;
break;
case PredicateType::InList:
@@ -293,7 +294,8 @@ int PushLowerBoundKeyPredicates(ColIdxIter first,
memcpy(row->mutable_cell_ptr(*col_idx_it), predicate->raw_lower(), size);
pushed_predicates++;
break;
- case PredicateType::IsNotNull:
+ case PredicateType::IsNotNull: // Fallthrough intended
+ case PredicateType::IsNull:
break_loop = true;
break;
case PredicateType::InList:
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/common/scan_spec.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/scan_spec.cc b/src/kudu/common/scan_spec.cc
index 3f3ad07..46acc08 100644
--- a/src/kudu/common/scan_spec.cc
+++ b/src/kudu/common/scan_spec.cc
@@ -73,6 +73,7 @@ void ScanSpec::SetLowerBoundKey(const EncodedKey* key) {
lower_bound_key_ = key;
}
}
+
void ScanSpec::SetExclusiveUpperBoundKey(const EncodedKey* key) {
if (exclusive_upper_bound_key_ == nullptr ||
key->encoded_key().compare(exclusive_upper_bound_key_->encoded_key()) < 0) {
http://git-wip-us.apache.org/repos/asf/kudu/blob/431aee53/src/kudu/common/wire_protocol.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/wire_protocol.cc b/src/kudu/common/wire_protocol.cc
index 361be0d..fd61231 100644
--- a/src/kudu/common/wire_protocol.cc
+++ b/src/kudu/common/wire_protocol.cc
@@ -385,6 +385,10 @@ void ColumnPredicateToPB(const ColumnPredicate& predicate,
pb->mutable_is_not_null();
return;
};
+ case PredicateType::IsNull: {
+ pb->mutable_is_null();
+ return;
+ }
case PredicateType::InList: {
auto* values = pb->mutable_in_list()->mutable_values();
for (const void* value : predicate.raw_values()) {
@@ -454,10 +458,13 @@ Status ColumnPredicateFromPB(const Schema& schema,
break;
};
case ColumnPredicatePB::kIsNotNull: {
- ColumnPredicate p = ColumnPredicate::IsNotNull(col);
*predicate = ColumnPredicate::IsNotNull(col);
break;
};
+ case ColumnPredicatePB::kIsNull: {
+ *predicate = ColumnPredicate::IsNull(col);
+ break;
+ }
default: return Status::InvalidArgument("Unknown predicate type for column", col.name());
}
return Status::OK();