You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by he...@apache.org on 2017/08/15 17:59:36 UTC

[1/7] incubator-impala git commit: IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

Repository: incubator-impala
Updated Branches:
  refs/heads/master f51c4435c -> b70acf92b


IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

Plumbing statuses through TextConverter::CodegenWriteSlot(), which
eventually propagate to the runtime profile.

Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
Reviewed-on: http://gerrit.cloudera.org:8080/7574
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/dfb158b4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/dfb158b4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/dfb158b4

Branch: refs/heads/master
Commit: dfb158b474afc26e761caf4b53ec4b4a48f842d3
Parents: f51c443
Author: aphadke <ap...@cloudera.com>
Authored: Wed Aug 2 18:47:56 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Aug 15 02:04:31 2017 +0000

----------------------------------------------------------------------
 be/src/exec/hdfs-scanner.cc   |  6 +--
 be/src/exec/text-converter.cc | 81 ++++++++++++++++++++++----------------
 be/src/exec/text-converter.h  |  7 ++--
 3 files changed, 53 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/dfb158b4/be/src/exec/hdfs-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scanner.cc b/be/src/exec/hdfs-scanner.cc
index 23fcc20..0ef62ab 100644
--- a/be/src/exec/hdfs-scanner.cc
+++ b/be/src/exec/hdfs-scanner.cc
@@ -320,11 +320,11 @@ Status HdfsScanner::CodegenWriteCompleteTuple(HdfsScanNodeBase* node,
   TupleDescriptor* tuple_desc = const_cast<TupleDescriptor*>(node->tuple_desc());
   vector<Function*> slot_fns;
   for (int i = 0; i < node->materialized_slots().size(); ++i) {
+    Function *fn = nullptr;
     SlotDescriptor* slot_desc = node->materialized_slots()[i];
-    Function* fn = TextConverter::CodegenWriteSlot(codegen, tuple_desc, slot_desc,
+    RETURN_IF_ERROR(TextConverter::CodegenWriteSlot(codegen, tuple_desc, slot_desc, &fn,
         node->hdfs_table()->null_column_value().data(),
-        node->hdfs_table()->null_column_value().size(), true, state->strict_mode());
-    if (fn == NULL) return Status("CodegenWriteSlot failed.");
+        node->hdfs_table()->null_column_value().size(), true, state->strict_mode()));
     if (i >= LlvmCodeGen::CODEGEN_INLINE_EXPRS_THRESHOLD) codegen->SetNoInline(fn);
     slot_fns.push_back(fn);
   }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/dfb158b4/be/src/exec/text-converter.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/text-converter.cc b/be/src/exec/text-converter.cc
index db390c4..431a11b 100644
--- a/be/src/exec/text-converter.cc
+++ b/be/src/exec/text-converter.cc
@@ -66,51 +66,52 @@ void TextConverter::UnescapeString(const char* src, char* dest, int* len,
 }
 
 // Codegen for a function to parse one slot.  The IR for a int slot looks like:
-// define i1 @WriteSlot({ i8, i32 }* %tuple_arg, i8* %data, i32 %len) {
+// define i1 @WriteSlot(<{ i32, i8 }>* %tuple_arg, i8* %data, i32 %len) #38 {
 // entry:
 //   %parse_result = alloca i32
-//   %0 = call i1 @IsNullString(i8* %data, i32 %len)
+//   %0 = call i1 @IrIsNullString(i8* %data, i32 %len)
 //   br i1 %0, label %set_null, label %check_zero
 //
 // set_null:                                         ; preds = %check_zero, %entry
-//   call void @SetNull({ i8, i32 }* %tuple_arg)
+//   %1 = bitcast <{ i32, i8 }>* %tuple_arg to i8*
+//   %null_byte_ptr2 = getelementptr inbounds i8, i8* %1, i32 4
+//   %null_byte3 = load i8, i8* %null_byte_ptr2
+//   %null_bit_set4 = or i8 %null_byte3, 1
+//   store i8 %null_bit_set4, i8* %null_byte_ptr2
 //   ret i1 true
 //
 // parse_slot:                                       ; preds = %check_zero
-//   %slot = getelementptr inbounds { i8, i32 }* %tuple_arg, i32 0, i32 1
-//   %1 = call i32 @IrStringToInt32(i8* %data, i32 %len, i32* %parse_result)
-//   %parse_result1 = load i32* %parse_result
+//   %slot = getelementptr inbounds <{ i32, i8 }>, <{ i32, i8 }>* %tuple_arg, i32 0, i32 0
+//   %2 = call i32 @IrStringToInt32(i8* %data, i32 %len, i32* %parse_result)
+//   %parse_result1 = load i32, i32* %parse_result
 //   %failed = icmp eq i32 %parse_result1, 1
 //   br i1 %failed, label %parse_fail, label %parse_success
 //
 // check_zero:                                       ; preds = %entry
-//   %2 = icmp eq i32 %len, 0
-//   br i1 %2, label %set_null, label %parse_slot
+//   %3 = icmp eq i32 %len, 0
+//   br i1 %3, label %set_null, label %parse_slot
 //
 // parse_success:                                    ; preds = %parse_slot
-//   store i32 %1, i32* %slot
+//   store i32 %2, i32* %slot
 //   ret i1 true
 //
 // parse_fail:                                       ; preds = %parse_slot
-//   call void @SetNull({ i8, i32 }* %tuple_arg)
+//   %4 = bitcast <{ i32, i8 }>* %tuple_arg to i8*
+//   %null_byte_ptr = getelementptr inbounds i8, i8* %4, i32 4
+//   %null_byte = load i8, i8* %null_byte_ptr
+//   %null_bit_set = or i8 %null_byte, 1
+//   store i8 %null_bit_set, i8* %null_byte_ptr
 //   ret i1 false
-// }
-//
-// If strict_mode = true, then 'parse_slot' also treats overflows errors, e.g.:
-// parse_slot:                                       ; preds = %check_zero
-//   %slot = getelementptr inbounds { i8, i32 }* %tuple_arg, i32 0, i32 1
-//   %1 = call i32 @IrStringToInt32(i8* %data, i32 %len, i32* %parse_result)
-//   %parse_result1 = load i32, i32* %parse_result
-//   %failed = icmp eq i32 %parse_result1, 1
-//   %overflowed = icmp eq i32 %parse_result1, 2
-//   %failed_or = or i1 %failed, %overflowed
-//   br i1 %failed_or, label %parse_fail, label %parse_success
-Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
-    TupleDescriptor* tuple_desc, SlotDescriptor* slot_desc,
+//}
+
+
+Status TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
+    TupleDescriptor* tuple_desc, SlotDescriptor* slot_desc, Function** fn,
     const char* null_col_val, int len, bool check_null, bool strict_mode) {
+  DCHECK(fn != nullptr);
   if (slot_desc->type().type == TYPE_CHAR) {
-    LOG(INFO) << "Char isn't supported for CodegenWriteSlot";
-    return NULL;
+    return Status("TextConverter::CodegenWriteSlot(): Char isn't supported for"
+        " CodegenWriteSlot");
   }
   SCOPED_TIMER(codegen->codegen_timer());
 
@@ -122,10 +123,14 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
   } else {
     is_null_string_fn = codegen->GetFunction(IRFunction::GENERIC_IS_NULL_STRING, false);
   }
-  if (is_null_string_fn == NULL) return NULL;
+
+  DCHECK(is_null_string_fn != NULL);
 
   StructType* tuple_type = tuple_desc->GetLlvmStruct(codegen);
-  if (tuple_type == NULL) return NULL;
+  if (tuple_type == NULL) {
+    return Status("TextConverter::CodegenWriteSlot(): Failed to generate "
+        "tuple type");
+  }
   PointerType* tuple_ptr_type = tuple_type->getPointerTo();
 
   LlvmCodeGen::FnPrototype prototype(
@@ -136,14 +141,14 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
 
   LlvmBuilder builder(codegen->context());
   Value* args[3];
-  Function* fn = prototype.GeneratePrototype(&builder, &args[0]);
+  *fn = prototype.GeneratePrototype(&builder, &args[0]);
 
   BasicBlock* set_null_block, *parse_slot_block, *check_zero_block = NULL;
-  codegen->CreateIfElseBlocks(fn, "set_null", "parse_slot",
+  codegen->CreateIfElseBlocks(*fn, "set_null", "parse_slot",
       &set_null_block, &parse_slot_block);
 
   if (!slot_desc->type().IsVarLenStringType()) {
-    check_zero_block = BasicBlock::Create(codegen->context(), "check_zero", fn);
+    check_zero_block = BasicBlock::Create(codegen->context(), "check_zero", *fn);
   }
 
   // Check if the data matches the configured NULL string.
@@ -175,7 +180,8 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
 
   // Codegen parse slot block
   builder.SetInsertPoint(parse_slot_block);
-  Value* slot = builder.CreateStructGEP(NULL, args[0], slot_desc->llvm_field_idx(), "slot");
+  Value* slot = builder.CreateStructGEP(NULL, args[0], slot_desc->llvm_field_idx(),
+      "slot");
 
   if (slot_desc->type().IsVarLenStringType()) {
     Value* ptr = builder.CreateStructGEP(NULL, slot, 0, "string_ptr");
@@ -225,17 +231,18 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
         break;
       default:
         DCHECK(false);
-        return NULL;
+        return Status("TextConverter::CodegenWriteSlot(): Codegen'd parser NYI for the"
+            "slot_desc type");
     }
     parse_fn = codegen->GetFunction(parse_fn_enum, false);
     DCHECK(parse_fn != NULL);
 
     // Set up trying to parse the string to the slot type
     BasicBlock* parse_success_block, *parse_failed_block;
-    codegen->CreateIfElseBlocks(fn, "parse_success", "parse_fail",
+    codegen->CreateIfElseBlocks(*fn, "parse_success", "parse_fail",
         &parse_success_block, &parse_failed_block);
     LlvmCodeGen::NamedVariable parse_result("parse_result", codegen->GetType(TYPE_INT));
-    Value* parse_result_ptr = codegen->CreateEntryBlockAlloca(fn, parse_result);
+    Value* parse_result_ptr = codegen->CreateEntryBlockAlloca(*fn, parse_result);
 
     Value* parse_return;
     // Call Impala's StringTo* function
@@ -281,5 +288,9 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
   slot_desc->CodegenSetNullIndicator(codegen, &builder, args[0], codegen->true_value());
   builder.CreateRet(codegen->true_value());
 
-  return codegen->FinalizeFunction(fn);
+  if (codegen->FinalizeFunction(*fn) == NULL) {
+    return Status("TextConverter::CodegenWriteSlot(): codegen'd "
+        "WriteSlot function failed verification, see log");
+  }
+  return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/dfb158b4/be/src/exec/text-converter.h
----------------------------------------------------------------------
diff --git a/be/src/exec/text-converter.h b/be/src/exec/text-converter.h
index 1a78af0..59d1968 100644
--- a/be/src/exec/text-converter.h
+++ b/be/src/exec/text-converter.h
@@ -70,7 +70,8 @@ class TextConverter {
   void UnescapeString(const char* src, char* dest, int* len, int64_t maxlen = -1);
 
   /// Codegen the function to write a slot for slot_desc.
-  /// Returns NULL if codegen was not succesful.
+  /// Returns Status::OK() if codegen was successful. If codegen was successful
+  /// llvm::Function** fn points to the codegen'd function
   /// The signature of the generated function is:
   /// bool WriteSlot(Tuple* tuple, const char* data, int len);
   /// The codegen function returns true if the slot could be written and false
@@ -81,8 +82,8 @@ class TextConverter {
   /// be used for partitions that contain escapes.
   /// strict_mode: If set, numerical overflow/underflow are considered to be parse
   /// errors.
-  static llvm::Function* CodegenWriteSlot(LlvmCodeGen* codegen,
-      TupleDescriptor* tuple_desc, SlotDescriptor* slot_desc,
+  static Status CodegenWriteSlot(LlvmCodeGen* codegen,
+      TupleDescriptor* tuple_desc, SlotDescriptor* slot_desc, llvm::Function** fn,
       const char* null_col_val, int len, bool check_null, bool strict_mode = false);
 
  private:


[4/7] incubator-impala git commit: IMPALA-5743: Support TLS version configuration for Thrift servers

Posted by he...@apache.org.
IMPALA-5743: Support TLS version configuration for Thrift servers

* Add --ssl_minimum_version which controls the minimum SSL/TLS version
  that clients and servers will use when negotiating a secure
  connection.
* Two kinds of version specification are allowed: 'TLSv1.1' enables
  TLSv1.1 and all subsequent verisons. 'TLSv1.1_only' enables only
  TLSv1.1. The latter is not exposed in user-facing text as it is
  typically only used for testing.
* Handle case where platform may not support TLSv1.1 or v1.2 by checking
  OpenSSL version number.
* Bump Thrift toolchain version to -p10.

Testing:
* New tests in thrift-server-test.cc. In particular, test all 36
  configurations of client and server protocol versions, and ensure that
  the expected successes or failures are seen.

Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5
Reviewed-on: http://gerrit.cloudera.org:8080/7606
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/16ce201f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/16ce201f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/16ce201f

Branch: refs/heads/master
Commit: 16ce201f5250451cb55e2cb9821b5d628d777160
Parents: 97803ce
Author: Henry Robinson <he...@cloudera.com>
Authored: Tue Aug 1 13:41:34 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Aug 15 09:28:05 2017 +0000

----------------------------------------------------------------------
 be/src/catalog/catalogd-main.cc            |  6 ++
 be/src/rpc/thrift-client.cc                | 14 ++++
 be/src/rpc/thrift-client.h                 |  6 +-
 be/src/rpc/thrift-server-test.cc           | 88 +++++++++++++++++++++++++
 be/src/rpc/thrift-server.cc                | 43 ++++++++++--
 be/src/rpc/thrift-server.h                 | 49 ++++++++++----
 be/src/service/impala-server.cc            | 19 ++++++
 be/src/statestore/statestore-subscriber.cc |  6 ++
 be/src/statestore/statestored-main.cc      |  6 ++
 bin/impala-config.sh                       |  4 +-
 10 files changed, 215 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/be/src/catalog/catalogd-main.cc
----------------------------------------------------------------------
diff --git a/be/src/catalog/catalogd-main.cc b/be/src/catalog/catalogd-main.cc
index 4847920..fe681d0 100644
--- a/be/src/catalog/catalogd-main.cc
+++ b/be/src/catalog/catalogd-main.cc
@@ -48,11 +48,13 @@ DECLARE_string(ssl_server_certificate);
 DECLARE_string(ssl_private_key);
 DECLARE_string(ssl_private_key_password_cmd);
 DECLARE_string(ssl_cipher_list);
+DECLARE_string(ssl_minimum_version);
 
 #include "common/names.h"
 
 using namespace impala;
 using namespace apache::thrift;
+using namespace apache::thrift;
 
 int CatalogdMain(int argc, char** argv) {
   FLAGS_webserver_port = 25020;
@@ -94,9 +96,13 @@ int CatalogdMain(int argc, char** argv) {
   ThriftServerBuilder builder("CatalogService", processor, FLAGS_catalog_service_port);
 
   if (EnableInternalSslConnections()) {
+    SSLProtocol ssl_version;
+    ABORT_IF_ERROR(
+        SSLProtoVersions::StringToProtocol(FLAGS_ssl_minimum_version, &ssl_version));
     LOG(INFO) << "Enabling SSL for CatalogService";
     builder.ssl(FLAGS_ssl_server_certificate, FLAGS_ssl_private_key)
         .pem_password_cmd(FLAGS_ssl_private_key_password_cmd)
+        .ssl_version(ssl_version)
         .cipher_list(FLAGS_ssl_cipher_list);
   }
   ABORT_IF_ERROR(builder.metrics(metrics.get()).Build(&server));

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/be/src/rpc/thrift-client.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-client.cc b/be/src/rpc/thrift-client.cc
index 6f01073..a64cdd3 100644
--- a/be/src/rpc/thrift-client.cc
+++ b/be/src/rpc/thrift-client.cc
@@ -23,6 +23,7 @@
 #include <thrift/Thrift.h>
 #include <gutil/strings/substitute.h>
 
+#include "util/network-util.h"
 #include "util/time.h"
 
 #include "common/names.h"
@@ -33,9 +34,22 @@ using namespace strings;
 
 DECLARE_string(ssl_client_ca_certificate);
 DECLARE_string(ssl_cipher_list);
+DECLARE_string(ssl_minimum_version);
 
 namespace impala {
 
+ThriftClientImpl::ThriftClientImpl(const std::string& ipaddress, int port, bool ssl)
+  : address_(MakeNetworkAddress(ipaddress, port)), ssl_(ssl) {
+  if (ssl_) {
+    SSLProtocol version;
+    socket_create_status_ =
+        SSLProtoVersions::StringToProtocol(FLAGS_ssl_minimum_version, &version);
+    if (!socket_create_status_.ok()) return;
+    ssl_factory_.reset(new TSSLSocketFactory(version));
+  }
+  socket_create_status_ = CreateSocket();
+}
+
 Status ThriftClientImpl::Open() {
   if (!socket_create_status_.ok()) return socket_create_status_;
   try {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/be/src/rpc/thrift-client.h
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-client.h b/be/src/rpc/thrift-client.h
index 26982a1..778075c 100644
--- a/be/src/rpc/thrift-client.h
+++ b/be/src/rpc/thrift-client.h
@@ -71,11 +71,7 @@ class ThriftClientImpl {
   Status socket_create_status() { return socket_create_status_; }
 
  protected:
-  ThriftClientImpl(const std::string& ipaddress, int port, bool ssl)
-      : address_(MakeNetworkAddress(ipaddress, port)), ssl_(ssl) {
-    if (ssl_) ssl_factory_.reset(new TSSLSocketFactory());
-    socket_create_status_ = CreateSocket();
-  }
+  ThriftClientImpl(const std::string& ipaddress, int port, bool ssl);
 
   /// Create a new socket without opening it. Returns an error if the socket could not
   /// be created.

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/be/src/rpc/thrift-server-test.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-server-test.cc b/be/src/rpc/thrift-server-test.cc
index a7c5ca5..67e149a 100644
--- a/be/src/rpc/thrift-server-test.cc
+++ b/be/src/rpc/thrift-server-test.cc
@@ -30,9 +30,11 @@
 using namespace impala;
 using namespace strings;
 using namespace apache::thrift;
+using apache::thrift::transport::SSLProtocol;
 
 DECLARE_string(ssl_client_ca_certificate);
 DECLARE_string(ssl_cipher_list);
+DECLARE_string(ssl_minimum_version);
 
 DECLARE_int32(state_store_port);
 
@@ -257,6 +259,92 @@ TEST(SslTest, MismatchedCiphers) {
       TTransportException);
 }
 
+// Test that StringToProtocol() correctly maps strings to their symbolic protocol
+// equivalents.
+TEST(SslTest, StringToProtocol) {
+  SSLProtocol version;
+#if OPENSSL_VERSION_NUMBER < 0x10001000L
+  // No TLSv1.1+ support in OpenSSL v1.0.0.
+  EXPECT_FALSE(SSLProtoVersions::StringToProtocol("tlsv1.2", &version).ok());
+  EXPECT_FALSE(SSLProtoVersions::StringToProtocol("tlsv1.1", &version).ok());
+  EXPECT_OK(SSLProtoVersions::StringToProtocol("tlsv1", &version));
+  EXPECT_EQ(TLSv1_0_plus, version);
+#else
+  map<string, SSLProtocol> TEST_CASES = {{"tlsv1", TLSv1_0_plus},
+      {"tlsv1.1", TLSv1_1_plus}, {"tlsv1.2", TLSv1_2_plus}, {"tlsv1_only", TLSv1_0},
+      {"tlsv1.1_only", TLSv1_1}, {"tlsv1.2_only", TLSv1_2}};
+  for (auto p : TEST_CASES) {
+    EXPECT_OK(SSLProtoVersions::StringToProtocol(p.first, &version));
+    EXPECT_EQ(p.second, version) << "TLS version: " << p.first;
+  }
+#endif
+}
+
+TEST(SslTest, TLSVersionControl) {
+  auto flag =
+      ScopedFlagSetter<string>::Make(&FLAGS_ssl_client_ca_certificate, SERVER_CERT);
+
+  // A config is really a pair (server_version, whitelist), where 'server_version' is the
+  // server TLS version to test, and 'whitelist' is the set of client protocols that
+  // should be able to connect successfully. This test tries all client protocols,
+  // expecting those in the whitelist to succeed, and those that are not to fail.
+  struct Config {
+    SSLProtocol server_version;
+    set<SSLProtocol> whitelist;
+  };
+
+#if OPENSSL_VERSION_NUMBER < 0x10001000L
+  vector<Config> configs = {
+      {TLSv1_0, {TLSv1_0, TLSv1_0_plus}}, {TLSv1_0_plus, {TLSv1_0, TLSv1_0_plus}}};
+#else
+  vector<Config> configs = {{TLSv1_0, {TLSv1_0, TLSv1_0_plus}},
+      {TLSv1_0_plus,
+          {TLSv1_0, TLSv1_1, TLSv1_2, TLSv1_0_plus, TLSv1_1_plus, TLSv1_2_plus}},
+      {TLSv1_1, {TLSv1_1_plus, TLSv1_1, TLSv1_0_plus}},
+      {TLSv1_1_plus, {TLSv1_1, TLSv1_2, TLSv1_0_plus, TLSv1_1_plus, TLSv1_2_plus}},
+      {TLSv1_2, {TLSv1_2, TLSv1_0_plus, TLSv1_1_plus, TLSv1_2_plus}},
+      {TLSv1_2_plus, {TLSv1_2, TLSv1_0_plus, TLSv1_1_plus, TLSv1_2_plus}}};
+#endif
+
+  for (const auto& config : configs) {
+    // For each config, start a server with the requested protocol spec, and then try to
+    // connect a client to it with every possible spec. This is an N^2 test, but the value
+    // of N is 6.
+    int port = GetServerPort();
+
+    ThriftServer* server;
+    EXPECT_OK(ThriftServerBuilder("DummyStatestore", MakeProcessor(), port)
+                  .ssl(SERVER_CERT, PRIVATE_KEY)
+                  .ssl_version(config.server_version)
+                  .Build(&server));
+    EXPECT_OK(server->Start());
+
+    for (auto client_version : SSLProtoVersions::PROTO_MAP) {
+      auto s = ScopedFlagSetter<string>::Make(
+          &FLAGS_ssl_minimum_version, client_version.first);
+      ThriftClient<StatestoreServiceClientWrapper> ssl_client(
+          "localhost", port, "", nullptr, true);
+      EXPECT_OK(ssl_client.Open());
+      bool send_done = false;
+      TRegisterSubscriberResponse resp;
+
+      if (config.whitelist.find(client_version.second) == config.whitelist.end()) {
+        EXPECT_THROW(ssl_client.iface()->RegisterSubscriber(
+                         resp, TRegisterSubscriberRequest(), &send_done),
+            TTransportException)
+            << "TLS version: " << config.server_version
+            << ", client version: " << client_version.first;
+      } else {
+        EXPECT_NO_THROW({
+          ssl_client.iface()->RegisterSubscriber(
+              resp, TRegisterSubscriberRequest(), &send_done);
+        }) << "TLS version: "
+           << config.server_version << ", client version: " << client_version.first;
+      }
+    }
+  }
+}
+
 TEST(SslTest, MatchedCiphers) {
   int port = GetServerPort();
   ThriftServer* server;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/be/src/rpc/thrift-server.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-server.cc b/be/src/rpc/thrift-server.cc
index e1e9a7f..1a558d9 100644
--- a/be/src/rpc/thrift-server.cc
+++ b/be/src/rpc/thrift-server.cc
@@ -36,10 +36,12 @@
 #include <sstream>
 #include "gen-cpp/Types_types.h"
 #include "rpc/TAcceptQueueServer.h"
+#include "rpc/auth-provider.h"
 #include "rpc/authentication.h"
 #include "rpc/thrift-server.h"
 #include "rpc/thrift-thread.h"
 #include "util/debug-util.h"
+#include "util/metrics.h"
 #include "util/network-util.h"
 #include "util/os-util.h"
 #include "util/uid-util.h"
@@ -58,8 +60,8 @@ using namespace apache::thrift::server;
 using namespace apache::thrift::transport;
 using namespace apache::thrift;
 
-DEFINE_int32(rpc_cnxn_attempts, 10, "Deprecated");
-DEFINE_int32(rpc_cnxn_retry_interval_ms, 2000, "Deprecated");
+DEFINE_int32_hidden(rpc_cnxn_attempts, 10, "Deprecated");
+DEFINE_int32_hidden(rpc_cnxn_retry_interval_ms, 2000, "Deprecated");
 
 DECLARE_bool(enable_accept_queue_server);
 DECLARE_string(principal);
@@ -70,6 +72,28 @@ DECLARE_string(ssl_cipher_list);
 
 namespace impala {
 
+// Specifies the allowed set of values for --ssl_minimum_version. To keep consistent with
+// Apache Kudu, specifying a single version enables all versions including and succeeding
+// that one (e.g. TLSv1.1 enables v1.1 and v1.2). Specifying TLSv1.1_only enables only
+// v1.1.
+map<string, SSLProtocol> SSLProtoVersions::PROTO_MAP = {
+#if OPENSSL_VERSION_NUMBER >= 0x10001000L
+    {"tlsv1.2", TLSv1_2_plus}, {"tlsv1.2_only", TLSv1_2}, {"tlsv1.1", TLSv1_1_plus},
+    {"tlsv1.1_only", TLSv1_1},
+#endif
+    {"tlsv1", TLSv1_0_plus}, {"tlsv1_only", TLSv1_0}};
+
+Status SSLProtoVersions::StringToProtocol(const string& in, SSLProtocol* protocol) {
+  for (const auto& proto : SSLProtoVersions::PROTO_MAP) {
+    if (iequals(in, proto.first)) {
+      *protocol = proto.second;
+      return Status::OK();
+    }
+  }
+
+  return Status(Substitute("Unknown TLS version: '$0'", in));
+}
+
 bool EnableInternalSslConnections() {
   // Enable SSL between servers only if both the client validation certificate and the
   // server certificate are specified. 'Client' here means clients that are used by Impala
@@ -328,11 +352,14 @@ ThriftServer::ThriftServer(const string& name,
   }
 }
 
+namespace {
+
 /// Factory subclass to override getPassword() which provides a password string to Thrift
 /// to decrypt the private key file.
 class ImpalaSslSocketFactory : public TSSLSocketFactory {
  public:
-  ImpalaSslSocketFactory(const string& password) : password_(password) { }
+  ImpalaSslSocketFactory(SSLProtocol version, const string& password)
+    : TSSLSocketFactory(version), password_(password) {}
 
  protected:
   virtual void getPassword(string& output, int size) {
@@ -344,13 +371,13 @@ class ImpalaSslSocketFactory : public TSSLSocketFactory {
   /// The password string.
   const string password_;
 };
-
+}
 Status ThriftServer::CreateSocket(boost::shared_ptr<TServerTransport>* socket) {
   if (ssl_enabled()) {
     // This 'factory' is only called once, since CreateSocket() is only called from
     // Start()
     boost::shared_ptr<TSSLSocketFactory> socket_factory(
-        new ImpalaSslSocketFactory(key_password_));
+        new ImpalaSslSocketFactory(version_, key_password_));
     socket_factory->overrideDefaultPasswordCallback();
     try {
       if (!cipher_list_.empty()) socket_factory->ciphers(cipher_list_);
@@ -367,8 +394,9 @@ Status ThriftServer::CreateSocket(boost::shared_ptr<TServerTransport>* socket) {
   }
 }
 
-Status ThriftServer::EnableSsl(const string& certificate, const string& private_key,
-    const string& pem_password_cmd, const std::string& ciphers) {
+Status ThriftServer::EnableSsl(SSLProtocol version, const string& certificate,
+    const string& private_key, const string& pem_password_cmd,
+    const std::string& ciphers) {
   DCHECK(!started_);
   if (certificate.empty()) return Status(TErrorCode::SSL_CERTIFICATE_PATH_BLANK);
   if (private_key.empty()) return Status(TErrorCode::SSL_PRIVATE_KEY_PATH_BLANK);
@@ -386,6 +414,7 @@ Status ThriftServer::EnableSsl(const string& certificate, const string& private_
   certificate_path_ = certificate;
   private_key_path_ = private_key;
   cipher_list_ = ciphers;
+  version_ = version;
 
   if (!pem_password_cmd.empty()) {
     if (!RunShellProcess(pem_password_cmd, &key_password_, true)) {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/be/src/rpc/thrift-server.h
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-server.h b/be/src/rpc/thrift-server.h
index e52edea..e486e37 100644
--- a/be/src/rpc/thrift-server.h
+++ b/be/src/rpc/thrift-server.h
@@ -18,21 +18,23 @@
 #ifndef IMPALA_RPC_THRIFT_SERVER_H
 #define IMPALA_RPC_THRIFT_SERVER_H
 
-#include <boost/thread/mutex.hpp>
 #include <boost/scoped_ptr.hpp>
 #include <boost/shared_ptr.hpp>
+#include <boost/thread/mutex.hpp>
 #include <boost/unordered_map.hpp>
 #include <boost/uuid/uuid_generators.hpp>
-#include <thrift/server/TServer.h>
 #include <thrift/TProcessor.h>
+#include <thrift/server/TServer.h>
+#include <thrift/transport/TSSLSocket.h>
 
 #include "common/status.h"
-#include "rpc/auth-provider.h"
 #include "util/metrics.h"
 #include "util/thread.h"
 
 namespace impala {
 
+class AuthProvider;
+
 /// Utility class for all Thrift servers. Runs a threaded server by default, or a
 /// TThreadPoolServer with, by default, 2 worker threads, that exposes the interface
 /// described by a user-supplied TProcessor object.
@@ -156,13 +158,15 @@ class ThriftServer {
       AuthProvider* auth_provider = nullptr, MetricGroup* metrics = nullptr,
       int num_worker_threads = DEFAULT_WORKER_THREADS, ServerType server_type = Threaded);
 
-  /// Enables secure access over SSL. Must be called before Start(). The first two
-  /// arguments are paths to certificate and private key files in .PEM format,
-  /// respectively. If either file does not exist, an error is returned. The final
-  /// optional argument provides the command to run if a password is required to decrypt
-  /// the private key. It is invoked once, and the resulting password is used only for
-  /// password-protected .PEM files.
-  Status EnableSsl(const std::string& certificate, const std::string& private_key,
+  /// Enables secure access over SSL. Must be called before Start(). The first three
+  /// arguments are the minimum SSL/TLS version, and paths to certificate and private key
+  /// files in .PEM format, respectively. If either file does not exist, an error is
+  /// returned. The fourth, optional, argument provides the command to run if a password
+  /// is required to decrypt the private key. It is invoked once, and the resulting
+  /// password is used only for password-protected .PEM files. The final argument is a
+  /// string containing a list of cipher suites, separated by commas, to enable.
+  Status EnableSsl(apache::thrift::transport::SSLProtocol version,
+      const std::string& certificate, const std::string& private_key,
       const std::string& pem_password_cmd = "", const std::string& ciphers = "");
 
   /// Creates the server socket on which this server listens. May be SSL enabled. Returns
@@ -191,6 +195,9 @@ class ThriftServer {
   /// List of ciphers that are ok for clients to use when connecting.
   std::string cipher_list_;
 
+  /// The SSL/TLS protocol client versions that this server will allow to connect.
+  apache::thrift::transport::SSLProtocol version_;
+
   /// How many worker threads to use to serve incoming requests
   /// (requests are queued if no thread is immediately available)
   int num_worker_threads_;
@@ -286,6 +293,12 @@ class ThriftServerBuilder {
     return *this;
   }
 
+  /// Sets the SSL/TLS client version(s) that this server will allow to connect.
+  ThriftServerBuilder& ssl_version(apache::thrift::transport::SSLProtocol version) {
+    version_ = version;
+    return *this;
+  }
+
   /// Sets the command used to compute the password for the SSL private key. Default is
   /// empty, i.e. no password needed.
   ThriftServerBuilder& pem_password_cmd(const std::string& pem_password_cmd) {
@@ -308,8 +321,8 @@ class ThriftServerBuilder {
     std::unique_ptr<ThriftServer> ptr(new ThriftServer(name_, processor_, port_,
         auth_provider_, metrics_, num_worker_threads_, server_type_));
     if (enable_ssl_) {
-      RETURN_IF_ERROR(
-          ptr->EnableSsl(certificate_, private_key_, pem_password_cmd_, ciphers_));
+      RETURN_IF_ERROR(ptr->EnableSsl(
+          version_, certificate_, private_key_, pem_password_cmd_, ciphers_));
     }
     (*server) = ptr.release();
     return Status::OK();
@@ -326,12 +339,24 @@ class ThriftServerBuilder {
   MetricGroup* metrics_ = nullptr;
 
   bool enable_ssl_ = false;
+  apache::thrift::transport::SSLProtocol version_ =
+      apache::thrift::transport::SSLProtocol::TLSv1_0;
   std::string certificate_;
   std::string private_key_;
   std::string pem_password_cmd_;
   std::string ciphers_;
 };
 
+/// Contains a map from string for --ssl_minimum_version to Thrift's SSLProtocol.
+struct SSLProtoVersions {
+  static std::map<std::string, apache::thrift::transport::SSLProtocol> PROTO_MAP;
+
+  /// Given a string, find a corresponding SSLProtocol from PROTO_MAP. Returns an error if
+  /// one cannot be found. Matching is case-insensitive.
+  static Status StringToProtocol(
+      const std::string& in, apache::thrift::transport::SSLProtocol* protocol);
+};
+
 // Returns true if, per the process configuration flags, server<->server communications
 // should use SSL.
 bool EnableInternalSslConnections();

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/be/src/service/impala-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 5778041..00a9d9a 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -96,6 +96,7 @@ using boost::system_time;
 using boost::uuids::random_generator;
 using boost::uuids::uuid;
 using namespace apache::thrift;
+using namespace apache::thrift::transport;
 using namespace boost::posix_time;
 using namespace beeswax;
 using namespace rapidjson;
@@ -178,6 +179,15 @@ DEFINE_string(ssl_cipher_list, "",
     "ciphers for more information. If empty, the default cipher list for your platform "
     "is used");
 
+const string SSL_MIN_VERSION_HELP = "The minimum SSL/TLS version that Thrift "
+    "services should use for both client and server connections. Supported versions are "
+#if OPENSSL_VERSION_NUMBER >= 0x10001000L
+    "TLSv1.0, TLSv1.1 and TLSv1.2";
+#else
+    "TLSv1.0";
+#endif
+DEFINE_string(ssl_minimum_version, "tlsv1", SSL_MIN_VERSION_HELP.c_str());
+
 DEFINE_int32(idle_session_timeout, 0, "The time, in seconds, that a session may be idle"
     " for before it is closed (and all running queries cancelled) by Impala. If 0, idle"
     " sessions are never expired.");
@@ -1933,6 +1943,12 @@ Status CreateImpalaServer(ExecEnv* exec_env, int beeswax_port, int hs2_port, int
 
   impala_server->reset(new ImpalaServer(exec_env));
 
+  SSLProtocol ssl_version = SSLProtocol::TLSv1_0;
+  if (!FLAGS_ssl_server_certificate.empty() || EnableInternalSslConnections()) {
+    RETURN_IF_ERROR(
+        SSLProtoVersions::StringToProtocol(FLAGS_ssl_minimum_version, &ssl_version));
+  }
+
   if (be_port != 0 && be_server != nullptr) {
     boost::shared_ptr<ImpalaInternalService> thrift_if(new ImpalaInternalService());
     boost::shared_ptr<TProcessor> be_processor(
@@ -1947,6 +1963,7 @@ Status CreateImpalaServer(ExecEnv* exec_env, int beeswax_port, int hs2_port, int
       LOG(INFO) << "Enabling SSL for backend";
       be_builder.ssl(FLAGS_ssl_server_certificate, FLAGS_ssl_private_key)
           .pem_password_cmd(FLAGS_ssl_private_key_password_cmd)
+          .ssl_version(ssl_version)
           .cipher_list(FLAGS_ssl_cipher_list);
     }
     RETURN_IF_ERROR(be_builder.metrics(exec_env->metrics()).Build(be_server));
@@ -1974,6 +1991,7 @@ Status CreateImpalaServer(ExecEnv* exec_env, int beeswax_port, int hs2_port, int
       LOG(INFO) << "Enabling SSL for Beeswax";
       builder.ssl(FLAGS_ssl_server_certificate, FLAGS_ssl_private_key)
           .pem_password_cmd(FLAGS_ssl_private_key_password_cmd)
+          .ssl_version(ssl_version)
           .cipher_list(FLAGS_ssl_cipher_list);
     }
     RETURN_IF_ERROR(
@@ -2000,6 +2018,7 @@ Status CreateImpalaServer(ExecEnv* exec_env, int beeswax_port, int hs2_port, int
       LOG(INFO) << "Enabling SSL for HiveServer2";
       builder.ssl(FLAGS_ssl_server_certificate, FLAGS_ssl_private_key)
           .pem_password_cmd(FLAGS_ssl_private_key_password_cmd)
+          .ssl_version(ssl_version)
           .cipher_list(FLAGS_ssl_cipher_list);
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/be/src/statestore/statestore-subscriber.cc
----------------------------------------------------------------------
diff --git a/be/src/statestore/statestore-subscriber.cc b/be/src/statestore/statestore-subscriber.cc
index 69ddfdc..7cfcaf7 100644
--- a/be/src/statestore/statestore-subscriber.cc
+++ b/be/src/statestore/statestore-subscriber.cc
@@ -39,6 +39,7 @@
 
 using boost::posix_time::seconds;
 using namespace apache::thrift;
+using namespace apache::thrift::transport;
 using namespace strings;
 
 DEFINE_int32(statestore_subscriber_timeout_seconds, 30, "The amount of time (in seconds)"
@@ -53,6 +54,7 @@ DECLARE_string(ssl_server_certificate);
 DECLARE_string(ssl_private_key);
 DECLARE_string(ssl_private_key_password_cmd);
 DECLARE_string(ssl_cipher_list);
+DECLARE_string(ssl_minimum_version);
 
 namespace impala {
 
@@ -196,9 +198,13 @@ Status StatestoreSubscriber::Start() {
     ThriftServerBuilder builder(
         "StatestoreSubscriber", processor, heartbeat_address_.port);
     if (EnableInternalSslConnections()) {
+      SSLProtocol ssl_version;
+      RETURN_IF_ERROR(
+          SSLProtoVersions::StringToProtocol(FLAGS_ssl_minimum_version, &ssl_version));
       LOG(INFO) << "Enabling SSL for Statestore subscriber";
       builder.ssl(FLAGS_ssl_server_certificate, FLAGS_ssl_private_key)
           .pem_password_cmd(FLAGS_ssl_private_key_password_cmd)
+          .ssl_version(ssl_version)
           .cipher_list(FLAGS_ssl_cipher_list);
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/be/src/statestore/statestored-main.cc
----------------------------------------------------------------------
diff --git a/be/src/statestore/statestored-main.cc b/be/src/statestore/statestored-main.cc
index f4c4672..1dcb682 100644
--- a/be/src/statestore/statestored-main.cc
+++ b/be/src/statestore/statestored-main.cc
@@ -43,11 +43,13 @@ DECLARE_string(ssl_server_certificate);
 DECLARE_string(ssl_private_key);
 DECLARE_string(ssl_private_key_password_cmd);
 DECLARE_string(ssl_cipher_list);
+DECLARE_string(ssl_minimum_version);
 
 #include "common/names.h"
 
 using namespace impala;
 using namespace apache::thrift;
+using namespace apache::thrift::transport;
 
 int StatestoredMain(int argc, char** argv) {
   // Override default for webserver port
@@ -87,9 +89,13 @@ int StatestoredMain(int argc, char** argv) {
   ThriftServer* server;
   ThriftServerBuilder builder("StatestoreService", processor, FLAGS_state_store_port);
   if (EnableInternalSslConnections()) {
+    SSLProtocol ssl_version;
+    ABORT_IF_ERROR(
+        SSLProtoVersions::StringToProtocol(FLAGS_ssl_minimum_version, &ssl_version));
     LOG(INFO) << "Enabling SSL for Statestore";
     builder.ssl(FLAGS_ssl_server_certificate, FLAGS_ssl_private_key)
         .pem_password_cmd(FLAGS_ssl_private_key_password_cmd)
+        .ssl_version(ssl_version)
         .cipher_list(FLAGS_ssl_cipher_list);
   }
   ABORT_IF_ERROR(builder.metrics(metrics.get()).Build(&server));

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/16ce201f/bin/impala-config.sh
----------------------------------------------------------------------
diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index 5470bb0..58b8529 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -72,7 +72,7 @@ fi
 # moving to a different build of the toolchain, e.g. when a version is bumped or a
 # compile option is changed. The build id can be found in the output of the toolchain
 # build jobs, it is constructed from the build number and toolchain git hash prefix.
-export IMPALA_TOOLCHAIN_BUILD_ID=447-f3bee11cc6
+export IMPALA_TOOLCHAIN_BUILD_ID=450-fc9954b4fa
 # Versions of toolchain dependencies.
 # -----------------------------------
 export IMPALA_AVRO_VERSION=1.7.4-p4
@@ -107,7 +107,7 @@ export IMPALA_SQUEASEL_VERSION=3.3
 # TPC utilities used for test/benchmark data generation.
 export IMPALA_TPC_DS_VERSION=2.1.0
 export IMPALA_TPC_H_VERSION=2.17.0
-export IMPALA_THRIFT_VERSION=0.9.0-p9
+export IMPALA_THRIFT_VERSION=0.9.0-p10
 export IMPALA_THRIFT_JAVA_VERSION=0.9.0
 export IMPALA_ZLIB_VERSION=1.2.8
 


[6/7] incubator-impala git commit: Fix link to Hadoop ADLS page

Posted by he...@apache.org.
Fix link to Hadoop ADLS page

At the time I added the original link, the
URL with /current2/ was correct. Now /current2/
no longer exists but /current3/ does. /current/
appears to be a stable alias that points to the
most recent /currentN/ directory so I used that
in the corrected link.

Change-Id: I5bcbf31735f398a879595846aeb1a6e1c7def9a2
Reviewed-on: http://gerrit.cloudera.org:8080/7672
Reviewed-by: Laurel Hale <la...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/8605784b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/8605784b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/8605784b

Branch: refs/heads/master
Commit: 8605784b73cc8e70940279aad5ed690deb986be4
Parents: aaec43b
Author: John Russell <jr...@cloudera.com>
Authored: Mon Aug 14 16:08:34 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Aug 15 17:41:30 2017 +0000

----------------------------------------------------------------------
 docs/topics/impala_adls.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/8605784b/docs/topics/impala_adls.xml
----------------------------------------------------------------------
diff --git a/docs/topics/impala_adls.xml b/docs/topics/impala_adls.xml
index d45f02e..a02525a 100644
--- a/docs/topics/impala_adls.xml
+++ b/docs/topics/impala_adls.xml
@@ -72,7 +72,7 @@ under the License.
         </li>
         <li>
           <p>
-            <xref href="https://hadoop.apache.org/docs/current2/hadoop-azure-datalake/index.html" scope="external" format="html">Hadoop Azure Data Lake Support</xref>
+            <xref href="https://hadoop.apache.org/docs/current/hadoop-azure-datalake/index.html" scope="external" format="html">Hadoop Azure Data Lake Support</xref>
           </p>
         </li>
       </ul>


[3/7] incubator-impala git commit: IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET.

Posted by he...@apache.org.
IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET.

Fix: If the source URI does not have write permissions we get
     a "READ WRITE warning" on CREATE TABLE LIKE PARQUET.
     During analyze() in the class CreateTableLikeFileStmt we should
     be checking only for "READ" permission for the source URI
     not for READ and WRITE as the source URI should be readable.

Change-Id: I56799c4da482fb634ce440f8764dd44234dc22ab
Reviewed-on: http://gerrit.cloudera.org:8080/7671
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/97803ce8
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/97803ce8
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/97803ce8

Branch: refs/heads/master
Commit: 97803ce8d2dc1b56f2a2ac2a2cbaaf72e0ad54ee
Parents: b9880ce
Author: Pranay <ps...@cloudera.com>
Authored: Mon Aug 14 15:09:42 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Aug 15 04:32:07 2017 +0000

----------------------------------------------------------------------
 .../java/org/apache/impala/analysis/CreateTableLikeFileStmt.java   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/97803ce8/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
index de2901f..9f17708 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
@@ -362,7 +362,7 @@ public class CreateTableLikeFileStmt extends CreateTableStmt {
       throw new AnalysisException("CREATE TABLE LIKE FILE statement is not supported " +
           "for Kudu tables.");
     }
-    schemaLocation_.analyze(analyzer, Privilege.ALL, FsAction.READ_WRITE);
+    schemaLocation_.analyze(analyzer, Privilege.ALL, FsAction.READ);
     switch (schemaFileFormat_) {
       case PARQUET:
         getColumnDefs().addAll(extractParquetSchema(schemaLocation_));


[2/7] incubator-impala git commit: IMPALA-1478: Improve error message when subquery is used in the ON clause

Posted by he...@apache.org.
IMPALA-1478: Improve error message when subquery is used in the
             ON clause

Fix: Print the error stating that "Suquery not allowed in the ON clause"
     Add test case for testing the failure when a subquery is used in
     the ON clause.

Change-Id: I0d1dc47987de7ea04402e1ead31d81cddf2f96f2
Reviewed-on: http://gerrit.cloudera.org:8080/7588
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/b9880cef
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/b9880cef
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/b9880cef

Branch: refs/heads/master
Commit: b9880cefbe97be2f5c7ed706a9b570c8b3234e34
Parents: dfb158b
Author: Pranay Singh <ps...@cloudera.com>
Authored: Fri Aug 4 10:42:08 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Aug 15 04:27:36 2017 +0000

----------------------------------------------------------------------
 fe/src/main/java/org/apache/impala/analysis/TableRef.java       | 4 ++++
 .../test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java  | 5 +++++
 2 files changed, 9 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b9880cef/fe/src/main/java/org/apache/impala/analysis/TableRef.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/TableRef.java b/fe/src/main/java/org/apache/impala/analysis/TableRef.java
index d5bb0fe..4dc9e34 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TableRef.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TableRef.java
@@ -530,6 +530,10 @@ public class TableRef implements ParseNode {
         throw new AnalysisException(
             "analytic expression not allowed in ON clause: " + toSql());
       }
+      if (onClause_.contains(Subquery.class)) {
+        throw new AnalysisException(
+            "Subquery is not allowed in ON clause: " + toSql());
+      }
       Set<TupleId> onClauseTupleIds = Sets.newHashSet();
       List<Expr> conjuncts = onClause_.getConjuncts();
       // Outer join clause conjuncts are registered for this particular table ref

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b9880cef/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
index cff7563..b22ca81 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
@@ -1355,6 +1355,11 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
         "join functional.alltypes b on (a.bigint_col = " +
         "lag(b.int_col) over(order by a.bigint_col))",
         "analytic expression not allowed in ON clause");
+    AnalysisError(
+        "select a.int_col from functional.alltypes a " +
+        "join functional.alltypes b on (a.id = b.id) and " +
+        "a.int_col < (select min(id) from functional.alltypes c)",
+        "Subquery is not allowed in ON clause");
     // unknown column
     AnalysisError(
         "select a.int_col from functional.alltypes a " +


[5/7] incubator-impala git commit: IMPALA-5116: Remove deprecated hash_* types in gutil

Posted by he...@apache.org.
IMPALA-5116: Remove deprecated hash_* types in gutil

The following class templates are substituted from C++11 standard.
__gnu_cxx::hash_map => std::unordered_map
__gnu_cxx::hash_set => std::unordered_set
__gnu_cxx::hash     => std::hash

Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Reviewed-on: http://gerrit.cloudera.org:8080/7414
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/aaec43b9
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/aaec43b9
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/aaec43b9

Branch: refs/heads/master
Commit: aaec43b9a1dc1b1a2fb97ae257147c4f6d78f27a
Parents: 16ce201
Author: Jinchul <ji...@gmail.com>
Authored: Thu Jul 13 23:21:46 2017 +0900
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Aug 15 16:20:57 2017 +0000

----------------------------------------------------------------------
 be/src/gutil/hash/hash.cc         |  13 ----
 be/src/gutil/hash/hash.h          | 133 +--------------------------------
 be/src/gutil/strings/join.h       |   6 --
 be/src/gutil/strings/serialize.cc |  17 ++---
 be/src/gutil/strings/serialize.h  |  17 ++---
 be/src/gutil/strings/split.cc     |  16 ++--
 be/src/gutil/strings/split.h      |  20 +++--
 7 files changed, 37 insertions(+), 185 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aaec43b9/be/src/gutil/hash/hash.cc
----------------------------------------------------------------------
diff --git a/be/src/gutil/hash/hash.cc b/be/src/gutil/hash/hash.cc
index beceab3..5a1988b 100644
--- a/be/src/gutil/hash/hash.cc
+++ b/be/src/gutil/hash/hash.cc
@@ -182,16 +182,3 @@ uint64 FingerprintInterleavedImplementation(const char *s, uint32 len) {
   mix(d, e, f);
   return CombineFingerprintHalves(c, f);
 }
-
-// Extern template definitions.
-
-#if defined(__GNUC__)
-#include <ext/hash_set>
-namespace __gnu_cxx {
-
-template class hash_set<std::string>;
-template class hash_map<std::string, std::string>;
-
-}  // namespace __gnu_cxx
-
-#endif

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aaec43b9/be/src/gutil/hash/hash.h
----------------------------------------------------------------------
diff --git a/be/src/gutil/hash/hash.h b/be/src/gutil/hash/hash.h
index f6a8b0a..ce17d43 100644
--- a/be/src/gutil/hash/hash.h
+++ b/be/src/gutil/hash/hash.h
@@ -77,12 +77,6 @@
 #include <stdint.h>     // for uintptr_t
 #include <string.h>
 #include <algorithm>
-#include <ext/hash_map>
-using __gnu_cxx::hash;
-using __gnu_cxx::hash_map;     // hacky way to make sure we import standard hash<> fns
-#include <ext/hash_set>
-using __gnu_cxx::hash;
-using __gnu_cxx::hash_set;
 #include <string>
 #include <utility>
 
@@ -98,32 +92,6 @@ using __gnu_cxx::hash_set;
 #include "gutil/hash/legacy_hash.h"
 #include "gutil/hash/string_hash.h"
 
-#include <ext/hash_set>
-namespace __gnu_cxx {
-
-
-// STLport and MSVC 10.0 above already define these.
-#if !defined(_STLP_LONG_LONG) && !(defined(_MSC_VER) && _MSC_VER >= 1600)
-
-#if defined(_MSC_VER)
-// MSVC's stl implementation with _MSC_VER less than 1600 doesn't have
-// this hash struct. STLport already defines this.
-template <typename T>
-struct hash {
-  size_t operator()(const T& t) const;
-};
-#endif  // defined(_MSC_VER)
-
-#endif  // !defined(_STLP_LONG_LONG) && !(defined(_MSC_VER) && _MSC_VER >= 1600)
-
-template<> struct hash<bool> {
-  size_t operator()(bool x) const { return static_cast<size_t>(x); }
-};
-
-
-}  // namespace __gnu_cxx
-
-
 
 // ----------------------------------------------------------------------
 // Fingerprint()
@@ -214,9 +182,7 @@ inline uint64 FingerprintCat(uint64 fp1, uint64 fp2) {
   return Hash64NumWithSeed(fp1, fp2);
 }
 
-#include <ext/hash_set>
-namespace __gnu_cxx {
-
+namespace std {
 
 // This intended to be a "good" hash function.  It may change from time to time.
 template<> struct hash<uint128> {
@@ -235,97 +201,25 @@ template<> struct hash<uint128> {
       return c;
     }
   }
-  // Less than operator for MSVC use.
-  bool operator()(const uint128& a, const uint128& b) const {
-    return a < b;
-  }
-  static const size_t bucket_size = 4;  // These are required by MSVC
-  static const size_t min_buckets = 8;  // 4 and 8 are defaults.
-};
-
-// Avoid collision with definition in port_hash.h (via port.h).
-#ifndef HAVE_DEFINED_HASH_FOR_POINTERS
-#define HAVE_DEFINED_HASH_FOR_POINTERS
-// Hash pointers as if they were int's, but bring more entropy to
-// the lower bits.
-template<class T> struct hash<T*> {
-  size_t operator()(T *x) const {
-    size_t k = reinterpret_cast<size_t>(x);
-    return k + (k >> 6);
-  }
-};
-#endif  // HAVE_DEFINED_HASH_FOR_POINTERS
-
-#if defined(__GNUC__)
-// Use our nice hash function for strings
-template<class _CharT, class _Traits, class _Alloc>
-struct hash<std::basic_string<_CharT, _Traits, _Alloc> > {
-  size_t operator()(const std::basic_string<_CharT, _Traits, _Alloc>& k) const {
-    return HashTo32(k.data(), static_cast<uint32>(k.length()));
-  }
-};
-
-// they don't define a hash for const string at all
-template<> struct hash<const std::string> {
-  size_t operator()(const std::string& k) const {
-    return HashTo32(k.data(), static_cast<uint32>(k.length()));
-  }
-};
-#endif  // defined(__GNUC__)
-
-// MSVC's STL requires an ever-so slightly different decl
-#if defined(STL_MSVC)
-template<> struct hash<char const*> {
-  size_t operator()(char const* const k) const {
-    return HashTo32(k, strlen(k));
-  }
-  // Less than operator:
-  bool operator()(char const* const a, char const* const b) const {
-    return strcmp(a, b) < 0;
-  }
-  static const size_t bucket_size = 4;  // These are required by MSVC
-  static const size_t min_buckets = 8;  // 4 and 8 are defaults.
-};
-
-// MSVC 10.0 and above have already defined this.
-#if !defined(_MSC_VER) || _MSC_VER < 1600
-template<> struct hash<std::string> {
-  size_t operator()(const std::string& k) const {
-    return HashTo32(k.data(), k.length());
-  }
-  // Less than operator:
-  bool operator()(const std::string& a, const std::string& b) const {
-    return a < b;
-  }
-  static const size_t bucket_size = 4;  // These are required by MSVC
   static const size_t min_buckets = 8;  // 4 and 8 are defaults.
 };
-#endif  // !defined(_MSC_VER) || _MSC_VER < 1600
-
-#endif  // defined(STL_MSVC)
 
 // Hasher for STL pairs. Requires hashers for both members to be defined
 template<class First, class Second>
 struct hash<pair<First, Second> > {
   size_t operator()(const pair<First, Second>& p) const {
-    size_t h1 = hash<First>()(p.first);
-    size_t h2 = hash<Second>()(p.second);
+    size_t h1 = std::hash<First>()(p.first);
+    size_t h2 = std::hash<Second>()(p.second);
     // The decision below is at compile time
     return (sizeof(h1) <= sizeof(uint32)) ?
             Hash32NumWithSeed(h1, h2)
             : Hash64NumWithSeed(h1, h2);
   }
-  // Less than operator for MSVC.
-  bool operator()(const pair<First, Second>& a,
-                  const pair<First, Second>& b) const {
-    return a < b;
-  }
-  static const size_t bucket_size = 4;  // These are required by MSVC
   static const size_t min_buckets = 8;  // 4 and 8 are defaults.
 };
 
 
-}  // namespace __gnu_cxx
+}  // namespace std
 
 
 // If you want an excellent string hash function, and you don't mind if it
@@ -397,23 +291,4 @@ struct GoodFastHash<const std::basic_string<_CharT, _Traits, _Alloc> > {
   static const size_t min_buckets = 8;  // 4 and 8 are defaults.
 };
 
-// Extern template declarations.
-//
-// gcc only for now.  msvc and others: this technique is likely to work with
-// your compiler too.  changelists welcome.
-//
-// This technique is limited to template specializations whose hash key
-// functions are declared in this file.
-
-#if defined(__GNUC__)
-#include <ext/hash_set>
-namespace __gnu_cxx {
-
-extern template class hash_set<std::string>;
-extern template class hash_map<std::string, std::string>;
-
-}  // namespace __gnu_cxx
-
-#endif  // defined(__GNUC__)
-
 #endif  // UTIL_HASH_HASH_H_

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aaec43b9/be/src/gutil/strings/join.h
----------------------------------------------------------------------
diff --git a/be/src/gutil/strings/join.h b/be/src/gutil/strings/join.h
index bd34c39..152629d 100644
--- a/be/src/gutil/strings/join.h
+++ b/be/src/gutil/strings/join.h
@@ -9,12 +9,6 @@
 
 #include <stdio.h>
 #include <string.h>
-#include <ext/hash_map>
-using __gnu_cxx::hash;
-using __gnu_cxx::hash_map;  // Not used in this file.
-#include <ext/hash_set>
-using __gnu_cxx::hash;
-using __gnu_cxx::hash_set;  // Not used in this file.
 #include <iterator>
 using std::back_insert_iterator;
 using std::iterator_traits;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aaec43b9/be/src/gutil/strings/serialize.cc
----------------------------------------------------------------------
diff --git a/be/src/gutil/strings/serialize.cc b/be/src/gutil/strings/serialize.cc
index a4e27c3..7088244 100644
--- a/be/src/gutil/strings/serialize.cc
+++ b/be/src/gutil/strings/serialize.cc
@@ -4,9 +4,6 @@
 
 #include <stddef.h>
 #include <stdlib.h>
-#include <ext/hash_map>
-using __gnu_cxx::hash;
-using __gnu_cxx::hash_map;
 #include <string>
 using std::string;
 #include <utility>
@@ -14,6 +11,8 @@ using std::make_pair;
 using std::pair;
 #include <vector>
 using std::vector;
+#include <unordered_map>
+using std::unordered_map;
 
 #include "gutil/casts.h"
 #include "gutil/integral_types.h"
@@ -221,7 +220,7 @@ int64 ReverseOrderedStringToInt64(const StringPiece& key) {
 //   and commas to separate entries.
 // --------------------------------------------------------------------------
 
-string DictionaryInt32Encode(const hash_map<string, int32>* dictionary) {
+string DictionaryInt32Encode(const unordered_map<string, int32>* dictionary) {
   vector<string> entries;
   for (const auto& entry : *dictionary) {
     entries.push_back(StringPrintf("%s:%d", entry.first.c_str(), entry.second));
@@ -232,7 +231,7 @@ string DictionaryInt32Encode(const hash_map<string, int32>* dictionary) {
   return result;
 }
 
-string DictionaryInt64Encode(const hash_map<string, int64>* dictionary) {
+string DictionaryInt64Encode(const unordered_map<string, int64>* dictionary) {
   vector<string> entries;
   for (const auto& entry : *dictionary) {
     entries.push_back(StringPrintf("%s:%" PRId64,
@@ -244,7 +243,7 @@ string DictionaryInt64Encode(const hash_map<string, int64>* dictionary) {
   return result;
 }
 
-string DictionaryDoubleEncode(const hash_map<string, double>* dictionary) {
+string DictionaryDoubleEncode(const unordered_map<string, double>* dictionary) {
   vector<string> entries;
   for (const auto& entry : *dictionary) {
     entries.push_back(StringPrintf("%s:%g", entry.first.c_str(), entry.second));
@@ -269,7 +268,7 @@ bool DictionaryParse(const string& encoded_str,
   return true;
 }
 
-bool DictionaryInt32Decode(hash_map<string, int32>* dictionary,
+bool DictionaryInt32Decode(unordered_map<string, int32>* dictionary,
                            const string& encoded_str) {
   vector<pair<string, string> > items;
   if (!DictionaryParse(encoded_str, &items))
@@ -288,7 +287,7 @@ bool DictionaryInt32Decode(hash_map<string, int32>* dictionary,
   return true;
 }
 
-bool DictionaryInt64Decode(hash_map<string, int64>* dictionary,
+bool DictionaryInt64Decode(unordered_map<string, int64>* dictionary,
                            const string& encoded_str) {
   vector<pair<string, string> > items;
   if (!DictionaryParse(encoded_str, &items))
@@ -308,7 +307,7 @@ bool DictionaryInt64Decode(hash_map<string, int64>* dictionary,
 }
 
 
-bool DictionaryDoubleDecode(hash_map<string, double>* dictionary,
+bool DictionaryDoubleDecode(unordered_map<string, double>* dictionary,
                             const string& encoded_str) {
   vector<pair<string, string> > items;
   if (!DictionaryParse(encoded_str, &items))

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aaec43b9/be/src/gutil/strings/serialize.h
----------------------------------------------------------------------
diff --git a/be/src/gutil/strings/serialize.h b/be/src/gutil/strings/serialize.h
index d69382a..1aad2eb 100644
--- a/be/src/gutil/strings/serialize.h
+++ b/be/src/gutil/strings/serialize.h
@@ -8,9 +8,6 @@
 #define STRINGS_SERIALIZE_H_
 
 #include <string.h>
-#include <ext/hash_map>
-using __gnu_cxx::hash;
-using __gnu_cxx::hash_map;
 #include <string>
 using std::string;
 #include <utility>
@@ -18,6 +15,8 @@ using std::make_pair;
 using std::pair;
 #include <vector>
 using std::vector;
+#include <unordered_map>
+using std::unordered_map;
 
 #include <common/logging.h>
 
@@ -328,15 +327,15 @@ bool DictionaryParse(const string& encoded_str,
 //   Note: these routines are not meant for use with very large dictionaries.
 //   They are written for convenience and not efficiency.
 // --------------------------------------------------------------------------
-string DictionaryInt32Encode(const hash_map<string, int32>* dictionary);
-string DictionaryInt64Encode(const hash_map<string, int64>* dictionary);
-string DictionaryDoubleEncode(const hash_map<string, double>* dictionary);
+string DictionaryInt32Encode(const unordered_map<string, int32>* dictionary);
+string DictionaryInt64Encode(const unordered_map<string, int64>* dictionary);
+string DictionaryDoubleEncode(const unordered_map<string, double>* dictionary);
 
-bool DictionaryInt32Decode(hash_map<string, int32>* dictionary,
+bool DictionaryInt32Decode(unordered_map<string, int32>* dictionary,
                            const string& encoded_str);
-bool DictionaryInt64Decode(hash_map<string, int64>* dictionary,
+bool DictionaryInt64Decode(unordered_map<string, int64>* dictionary,
                            const string& encoded_str);
-bool DictionaryDoubleDecode(hash_map<string, double>* dictionary,
+bool DictionaryDoubleDecode(unordered_map<string, double>* dictionary,
                             const string& encoded_str);
 
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aaec43b9/be/src/gutil/strings/split.cc
----------------------------------------------------------------------
diff --git a/be/src/gutil/strings/split.cc b/be/src/gutil/strings/split.cc
index 34a9d99..e888856 100644
--- a/be/src/gutil/strings/split.cc
+++ b/be/src/gutil/strings/split.cc
@@ -118,7 +118,7 @@ namespace {
 // the following overloads:
 // - vector<string>           - for better performance
 // - map<string, string>      - to change append semantics
-// - hash_map<string, string> - to change append semantics
+// - unordered_map<string, string> - to change append semantics
 template <typename Container, typename Splitter>
 void AppendToImpl(Container* container, Splitter splitter) {
   Container c = splitter;  // Calls implicit conversion operator.
@@ -138,7 +138,7 @@ void AppendToImpl(vector<string>* container, Splitter splitter) {
   }
 }
 
-// Here we define two AppendToImpl() overloads for map<> and hash_map<>. Both of
+// Here we define two AppendToImpl() overloads for map<> and unordered_map<>. Both of
 // these overloads call through to this AppendToMap() function. This is needed
 // because inserting a duplicate key into a map does NOT overwrite the previous
 // value, which was not the behavior of the split1 Split*() functions. Consider
@@ -150,7 +150,7 @@ void AppendToImpl(vector<string>* container, Splitter splitter) {
 //   ASSERT_EQ(m["a"], "1");  // <-- "a" has value "1" not "2".
 //
 // Due to this behavior of map::insert, we can't rely on a normal std::inserter
-// for a maps. Instead, maps and hash_maps need to be special cased to implement
+// for a maps. Instead, maps and unordered_maps need to be special cased to implement
 // the desired append semantic of inserting an existing value overwrites the
 // previous value.
 //
@@ -172,7 +172,7 @@ void AppendToImpl(map<string, string>* map_container, Splitter splitter) {
 }
 
 template <typename Splitter>
-void AppendToImpl(hash_map<string, string>* map_container, Splitter splitter) {
+void AppendToImpl(unordered_map<string, string>* map_container, Splitter splitter) {
   AppendToMap(map_container, splitter);
 }
 
@@ -420,7 +420,7 @@ void SplitStringUsing(const string& full,
 }
 
 void SplitStringToHashsetUsing(const string& full, const char* delim,
-                               hash_set<string>* result) {
+                               unordered_set<string>* result) {
   AppendTo(result, strings::Split(full, AnyOf(delim), strings::SkipEmpty()));
 }
 
@@ -435,7 +435,7 @@ void SplitStringToMapUsing(const string& full, const char* delim,
 }
 
 void SplitStringToHashmapUsing(const string& full, const char* delim,
-                               hash_map<string, string>* result) {
+                               unordered_map<string, string>* result) {
   AppendTo(result, strings::Split(full, AnyOf(delim), strings::SkipEmpty()));
 }
 
@@ -588,8 +588,8 @@ void SplitStringWithEscapingToSet(const string &full,
 
 void SplitStringWithEscapingToHashset(const string &full,
                                       const strings::CharSet& delimiters,
-                                      hash_set<string> *result) {
-  std::insert_iterator< hash_set<string> > it(*result, result->end());
+                                      unordered_set<string> *result) {
+  std::insert_iterator< unordered_set<string> > it(*result, result->end());
   SplitStringWithEscapingToIterator(full, delimiters, false, &it);
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aaec43b9/be/src/gutil/strings/split.h
----------------------------------------------------------------------
diff --git a/be/src/gutil/strings/split.h b/be/src/gutil/strings/split.h
index e2f74a7..964e3fc 100644
--- a/be/src/gutil/strings/split.h
+++ b/be/src/gutil/strings/split.h
@@ -28,12 +28,6 @@ using std::min;
 using std::reverse;
 using std::sort;
 using std::swap;
-#include <ext/hash_map>
-using __gnu_cxx::hash;
-using __gnu_cxx::hash_map;
-#include <ext/hash_set>
-using __gnu_cxx::hash;
-using __gnu_cxx::hash_set;
 #include <iterator>
 using std::back_insert_iterator;
 using std::iterator_traits;
@@ -50,6 +44,10 @@ using std::make_pair;
 using std::pair;
 #include <vector>
 using std::vector;
+#include <unordered_map>
+using std::unordered_map;
+#include <unordered_set>
+using std::unordered_set;
 
 #include <common/logging.h>
 
@@ -131,8 +129,8 @@ namespace strings {
 // string, StringPiece, Cord, or any object that has a constructor (explicit or
 // not) that takes a single StringPiece argument. This pattern works for all
 // standard STL containers including vector, list, deque, set, multiset, map,
-// and multimap, non-standard containers including hash_set and hash_map, and
-// even std::pair which is not actually a container.
+// multimap, unordered_set and unordered_map, and even std::pair which is not
+// actually a container.
 //
 // Splitting to std::pair is an interesting case because it can hold only two
 // elements and is not a collection type. When splitting to an std::pair the
@@ -679,7 +677,7 @@ void SplitStringPieceToVector(const StringPiece& full,
 void SplitStringUsing(const string& full, const char* delimiters,
                       vector<string>* result);
 void SplitStringToHashsetUsing(const string& full, const char* delimiters,
-                               hash_set<string>* result);
+                               unordered_set<string>* result);
 void SplitStringToSetUsing(const string& full, const char* delimiters,
                            set<string>* result);
 // The even-positioned (0-based) components become the keys for the
@@ -690,7 +688,7 @@ void SplitStringToSetUsing(const string& full, const char* delimiters,
 void SplitStringToMapUsing(const string& full, const char* delim,
                            map<string, string>* result);
 void SplitStringToHashmapUsing(const string& full, const char* delim,
-                               hash_map<string, string>* result);
+                               unordered_map<string, string>* result);
 
 // ----------------------------------------------------------------------
 // SplitStringAllowEmpty()
@@ -744,7 +742,7 @@ void SplitStringWithEscapingToSet(const string& full,
                                   set<string>* result);
 void SplitStringWithEscapingToHashset(const string& full,
                                       const strings::CharSet& delimiters,
-                                      hash_set<string>* result);
+                                      unordered_set<string>* result);
 
 // ----------------------------------------------------------------------
 // SplitStringIntoNPiecesAllowEmpty()


[7/7] incubator-impala git commit: [DOCS] Fold some lines

Posted by he...@apache.org.
[DOCS] Fold some <codeblock> lines

Putting <codeblock> on a separate line causes an extra
leading blank line in PDF output. (I think that's a
bug in the PDF transform, but what can you do...)

I took one file that had a few instances of such
examples, and folded the first line of the code
example onto the same line as the <codeblock> tag.

Using this minor change to demo upstream -> downstream
cherry-picking techniques.

Change-Id: I563a71ab9e59c691264c1f6a088e71f4722d3097
Reviewed-on: http://gerrit.cloudera.org:8080/7417
Reviewed-by: Michael Brown <mi...@cloudera.com>
Reviewed-by: Laurel Hale <la...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/b70acf92
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/b70acf92
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/b70acf92

Branch: refs/heads/master
Commit: b70acf92bfe7acf69775818cc16369b7527dd5e2
Parents: 8605784
Author: John Russell <jr...@cloudera.com>
Authored: Thu Jul 13 13:12:04 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Aug 15 17:43:34 2017 +0000

----------------------------------------------------------------------
 docs/topics/impala_connecting.xml | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b70acf92/docs/topics/impala_connecting.xml
----------------------------------------------------------------------
diff --git a/docs/topics/impala_connecting.xml b/docs/topics/impala_connecting.xml
index 9ac9a6a..b6b4928 100644
--- a/docs/topics/impala_connecting.xml
+++ b/docs/topics/impala_connecting.xml
@@ -89,8 +89,7 @@ Lots of nuances to illustrate through sample code.
         Use the <codeph>-i</codeph> option to the
         <cmdname>impala-shell</cmdname> interpreter to specify the connection information for
         that instance of <cmdname>impalad</cmdname>:
-<codeblock>
-# When you are logged into the same machine running impalad.
+<codeblock># When you are logged into the same machine running impalad.
 # The prompt will reflect the current hostname.
 $ impala-shell
 
@@ -166,8 +165,7 @@ $ impala-shell -i <varname>some.other.hostname</varname>:<varname>port_number</v
         <cmdname>impala-shell</cmdname> interpreter to connect and immediately
         switch to the specified database, without the need for a <codeph>USE</codeph>
         statement or fully qualified names:
-<codeblock>
-# Subsequent queries with unqualified names operate on
+<codeblock># Subsequent queries with unqualified names operate on
 # tables, views, and so on inside the database named 'staging'.
 $ impala-shell -i localhost -d staging
 
@@ -202,8 +200,7 @@ $ impala-shell -i localhost -d parquet_gzip_compression
         the <codeph>-f</codeph> option to run a sequence of statements from a file.
         The <cmdname>impala-shell</cmdname> command returns immediately, without going into
         the interactive interpreter.
-<codeblock>
-# A utility command that you might run while developing shell scripts
+<codeblock># A utility command that you might run while developing shell scripts
 # to manipulate HDFS files.
 $ impala-shell -i localhost -d database_of_interest -q 'show tables'