You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/05/28 23:09:11 UTC

[impala] branch master updated (3bbb855 -> 7ea8c57)

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

tarmstrong pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.


    from 3bbb855  IMPALA-8585: Fix StmtMetadataLoaderTest when compiled against Hive 3
     new 625dd52  IMPALA-8369: Skip test_max_nesting_depth with Hive 3
     new 7ea8c57  IMPALA-5031: Out-of-range enum values are undefined behavior

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 be/src/service/impala-hs2-server.cc   | 19 ++++++++++++++-----
 tests/common/skip.py                  |  2 ++
 tests/query_test/test_nested_types.py |  4 +++-
 3 files changed, 19 insertions(+), 6 deletions(-)


[impala] 01/02: IMPALA-8369: Skip test_max_nesting_depth with Hive 3

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 625dd524cae2cadbcd8f05748e43e9eb9374c9ae
Author: Csaba Ringhofer <cs...@cloudera.com>
AuthorDate: Mon May 27 18:43:16 2019 +0200

    IMPALA-8369: Skip test_max_nesting_depth with Hive 3
    
    The test timed out with CDP Hive because Hive hung
    during inserting to the Orc table from the Parquet
    table. See HIVE-21796 for details about the Hive
    issue.
    
    Change-Id: I44c0a731e53d7f4f31111201a3ab115e7cff9e52
    Reviewed-on: http://gerrit.cloudera.org:8080/13445
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 tests/common/skip.py                  | 2 ++
 tests/query_test/test_nested_types.py | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/common/skip.py b/tests/common/skip.py
index fea3f24..dba8c2d 100644
--- a/tests/common/skip.py
+++ b/tests/common/skip.py
@@ -202,6 +202,8 @@ class SkipIfDockerizedCluster:
 class SkipIfHive3:
   sentry_not_supported = pytest.mark.skipif(HIVE_MAJOR_VERSION >= 3,
       reason="Sentry HMS follower does not work with HMS-3. See SENTRY-2518 for details")
+  slow_nested_types = pytest.mark.skipif(HIVE_MAJOR_VERSION >= 3,
+      reason="Deeply nested types can be slow in Hive 3. See HIVE-21796 for details")
 
 
 class SkipIfHive2:
diff --git a/tests/query_test/test_nested_types.py b/tests/query_test/test_nested_types.py
index 0fe3869..bd18f35 100644
--- a/tests/query_test/test_nested_types.py
+++ b/tests/query_test/test_nested_types.py
@@ -32,7 +32,8 @@ from tests.common.skip import (
     SkipIfADLS,
     SkipIfEC,
     SkipIfLocal,
-    SkipIfNotHdfsMinicluster
+    SkipIfNotHdfsMinicluster,
+    SkipIfHive3
     )
 from tests.common.test_vector import ImpalaTestDimension
 from tests.util.filesystem_utils import WAREHOUSE, get_fs_path, IS_HDFS
@@ -609,6 +610,7 @@ class TestMaxNestingDepth(ImpalaTestSuite):
     cls.ImpalaTestMatrix.add_constraint(lambda v:
         v.get_value('table_format').file_format in ['parquet', 'orc'])
 
+  @SkipIfHive3.slow_nested_types
   def test_max_nesting_depth(self, vector, unique_database):
     """Tests that Impala can scan Parquet and ORC files having complex types of
     the maximum nesting depth."""


[impala] 02/02: IMPALA-5031: Out-of-range enum values are undefined behavior

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 7ea8c5706db5d2e7424d687465d702f01eef8824
Author: Jim Apple <jb...@apache.org>
AuthorDate: Sat May 25 23:06:52 2019 -0700

    IMPALA-5031: Out-of-range enum values are undefined behavior
    
    This patch handles an undefined behavior in the custom cluster tests
    in which a reference to an invalid enum value is used. To prevent the
    invalid value, the bytes are first copied into their underlying
    integer type.
    
    The undefined behavior happens in LdapJdbcTest. The relevant backtrace
    is:
    
        include/c++/4.9.2/bits/stl_algobase.h:199:11: runtime error: load
           of value 8, which is not a valid value for type 'const
           TProtocolVersion::type'
        #0 TProtocolVersion::type const&
           min<TProtocolVersion::type>(TProtocolVersion::type const&,
           TProtocolVersion::type const&)
           include/c++/4.9.2/bits/stl_algobase.h:199:11
        #1 ImpalaServer::OpenSession(TOpenSessionResp&, TOpenSessionReq
           const&) service/impala-hs2-server.cc:304:24
        #2 TCLIServiceProcessor::process_OpenSession(int, TProtocol*,
           TProtocol*, void*)
           generated-sources/gen-cpp/TCLIService.cpp:4953:13
        #3 TCLIServiceProcessor::dispatchCall(TProtocol*, TProtocol*,
           string const&, int, void*)
           generated-sources/gen-cpp/TCLIService.cpp:4926:3
        #4 ImpalaHiveServer2ServiceProcessor::dispatchCall(TProtocol*,
           TProtocol*, string const&, int, void*)
           generated-sources/gen-cpp/ImpalaHiveServer2Service.cpp:505:73
        #5 thrift::TDispatchProcessor::process
           (boost::shared_ptr<TProtocol>, boost::shared_ptr<TProtocol>,
           void*)
           toolchain/thrift-0.9.3-p5/include/thrift/TDispatchProcessor.h:121:12
        #6 thrift::server::TAcceptQueueServer::Task::run()
           rpc/TAcceptQueueServer.cpp:74:26
        #7 ThriftThread::RunRunnable(boost::shared_ptr
           <thrift::concurrency::Runnable>, Promise<unsigned long,
           (PromiseMode)0>*) rpc/thrift-thread.cc:74:13
    
    Change-Id: I63379b4c9d2e4738e729a556108c77fed85e6b64
    Reviewed-on: http://gerrit.cloudera.org:8080/13438
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/service/impala-hs2-server.cc | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/be/src/service/impala-hs2-server.cc b/be/src/service/impala-hs2-server.cc
index df1c093..4233f22 100644
--- a/be/src/service/impala-hs2-server.cc
+++ b/be/src/service/impala-hs2-server.cc
@@ -20,17 +20,19 @@
 #include "service/impala-server.inline.h"
 
 #include <algorithm>
+#include <type_traits>
+
+#include <boost/algorithm/string.hpp>
 #include <boost/algorithm/string/join.hpp>
+#include <boost/bind.hpp>
 #include <boost/date_time/posix_time/posix_time_types.hpp>
 #include <boost/unordered_set.hpp>
 #include <jni.h>
-#include <thrift/protocol/TDebugProtocol.h>
-#include <gtest/gtest.h>
-#include <boost/bind.hpp>
-#include <boost/algorithm/string.hpp>
 #include <gperftools/heap-profiler.h>
 #include <gperftools/malloc_extension.h>
+#include <gtest/gtest.h>
 #include <gutil/strings/substitute.h>
+#include <thrift/protocol/TDebugProtocol.h>
 
 #include "common/logging.h"
 #include "common/version.h"
@@ -301,7 +303,14 @@ void ImpalaServer::OpenSession(TOpenSessionResp& return_val,
   state->session_type = TSessionType::HIVESERVER2;
   state->network_address = ThriftServer::GetThreadConnectionContext()->network_address;
   state->last_accessed_ms = UnixMillis();
-  state->hs2_version = min(MAX_SUPPORTED_HS2_VERSION, request.client_protocol);
+  // request.client_protocol is not guaranteed to be a valid TProtocolVersion::type, so
+  // loading it can cause undefined behavior. Instead, we copy it to a value of the
+  // "underlying type" of the enum, then copy it back to state->hs_version only once we
+  // have clamped it to be at most MAX_SUPPORTED_HS2_VERSION.
+  std::underlying_type_t<decltype(request.client_protocol)> protocol_integer;
+  memcpy(&protocol_integer, &request.client_protocol, sizeof(request.client_protocol));
+  state->hs2_version = static_cast<TProtocolVersion::type>(
+      min<decltype(protocol_integer)>(MAX_SUPPORTED_HS2_VERSION, protocol_integer));
   state->kudu_latest_observed_ts = 0;
 
   // If the username was set by a lower-level transport, use it.