You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2022/06/13 17:15:23 UTC

[impala] branch master updated (4b5b0a693 -> 1285fc95a)

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

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


    from 4b5b0a693 IMPALA-11334: Include partition transform information in DESCRIBE FORMATTED for Iceberg tables
     new cee086f68 IMPALA-11302, IMPALA-11303: Fix error handling of external Iceberg tables
     new c7784bde5 IMPALA-1682: Support printing the output of a query (rows) vertically.
     new 5263d1311 IMPALA-11314: Test PyPI package with system python
     new 181fd9406 IMPALA-8373: Test impala-shell with python3
     new 1285fc95a IMPALA-11343: Override isOpen to avoid thrift bug

The 5 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:
 CMakeLists.txt                                     | 13 +--
 bin/bootstrap_build.sh                             |  2 +-
 bin/bootstrap_system.sh                            |  4 +-
 bin/impala-shell.sh                                |  3 +-
 bin/{impala-flake8 => impala-virtualenv}           |  2 +-
 docs/topics/impala_shell_options.xml               | 16 ++++
 .../apache/impala/analysis/CreateTableStmt.java    | 29 ++++++-
 shell/CMakeLists.txt                               | 54 ++++++++++++
 shell/TSSLSocketWithWildcardSAN.py                 |  4 +
 shell/impala_shell.py                              |  7 +-
 shell/impala_shell_config_defaults.py              |  1 +
 shell/option_parser.py                             |  4 +
 shell/shell_output.py                              | 36 ++++++--
 .../queries/QueryTest/iceberg-create.test          |  9 --
 .../queries/QueryTest/iceberg-negative.test        | 17 ++++
 tests/custom_cluster/test_client_ssl.py            |  3 +-
 tests/query_test/test_date_queries.py              |  3 +-
 tests/shell/test_shell_commandline.py              | 46 +++++-----
 tests/shell/test_shell_interactive.py              | 53 +++++-------
 tests/shell/util.py                                | 97 ++++++++++------------
 20 files changed, 265 insertions(+), 138 deletions(-)
 copy bin/{impala-flake8 => impala-virtualenv} (92%)
 create mode 100644 shell/CMakeLists.txt


[impala] 03/05: IMPALA-11314: Test PyPI package with system python

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

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

commit 5263d13112bc35ae69e2c2c09adc823a96fc8c13
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Thu Jun 2 10:02:23 2022 -0700

    IMPALA-11314: Test PyPI package with system python
    
    Sets up a virtualenv with system python to install the impala-shell PyPI
    package into. Using system python provides better coverage for Python
    versions likely to be used by customers. Runs impala-shell tests using
    the PyPI package to provide better coverage for the artifact customers
    will use.
    
    Includes a PyPI install in notests_independent_targets because these
    seem to be used for Python testing despite -notests.
    
    Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
    Reviewed-on: http://gerrit.cloudera.org:8080/18586
    Reviewed-by: Joe McDonnell <jo...@cloudera.com>
    Tested-by: Joe McDonnell <jo...@cloudera.com>
---
 CMakeLists.txt                          | 11 +---
 bin/impala-shell.sh                     |  3 +-
 bin/impala-virtualenv                   | 21 ++++++++
 shell/CMakeLists.txt                    | 45 ++++++++++++++++
 tests/custom_cluster/test_client_ssl.py |  3 +-
 tests/query_test/test_date_queries.py   |  3 +-
 tests/shell/test_shell_commandline.py   | 28 +++++-----
 tests/shell/test_shell_interactive.py   | 40 ++++----------
 tests/shell/util.py                     | 96 +++++++++++++++------------------
 9 files changed, 140 insertions(+), 110 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 857d295d6..30280b12e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -451,6 +451,7 @@ add_subdirectory(common/protobuf)
 add_subdirectory(be)
 add_subdirectory(docker)
 add_subdirectory(java)
+add_subdirectory(shell)
 
 # Build target for all generated files which most backend code depends on
 add_custom_target(gen-deps ALL DEPENDS thrift-deps proto-deps fb-deps
@@ -458,14 +459,6 @@ add_custom_target(gen-deps ALL DEPENDS thrift-deps proto-deps fb-deps
 
 add_custom_target(tarballs ALL DEPENDS shell_tarball)
 
-add_custom_target(shell_tarball DEPENDS gen-deps
-  COMMAND "${CMAKE_SOURCE_DIR}/shell/make_shell_tarball.sh"
-)
-
-add_custom_target(shell_pypi_package DEPENDS shell_tarball
-  COMMAND "${CMAKE_SOURCE_DIR}/shell/packaging/make_python_package.sh"
-)
-
 add_custom_target(cscope ALL DEPENDS gen-deps
   COMMAND "${CMAKE_SOURCE_DIR}/bin/gen-cscope.sh"
 )
@@ -475,7 +468,7 @@ add_custom_target(impala_python ALL
 )
 
 add_custom_target(notests_independent_targets DEPENDS
-  java cscope tarballs impala_python
+  java cscope tarballs impala_python shell_python2_install
 )
 add_custom_target(notests_regular_targets DEPENDS
   impalad statestored catalogd admissiond fesupport loggingsupport ImpalaUdf udasample udfsample impala-profile-tool
diff --git a/bin/impala-shell.sh b/bin/impala-shell.sh
index 7181d50c8..65f6b6b30 100755
--- a/bin/impala-shell.sh
+++ b/bin/impala-shell.sh
@@ -38,7 +38,8 @@ export LD_LIBRARY_PATH=":$(PYTHONPATH=${PYTHONPATH} \
 
 IMPALA_PY_DIR="$(dirname "$0")/../infra/python"
 IMPALA_PY_ENV_DIR="${IMPALA_PY_DIR}/env-gcc${IMPALA_GCC_VERSION}"
-IMPALA_PYTHON_EXECUTABLE="${IMPALA_PY_ENV_DIR}/bin/python"
+# Allow overriding the python executable
+IMPALA_PYTHON_EXECUTABLE="${IMPALA_PYTHON_EXECUTABLE:-${IMPALA_PY_ENV_DIR}/bin/python}"
 
 for PYTHON_LIB_DIR in ${THRIFT_PY_ROOT}/python/lib{64,}; do
   [[ -d ${PYTHON_LIB_DIR} ]] || continue
diff --git a/bin/impala-virtualenv b/bin/impala-virtualenv
new file mode 100755
index 000000000..8f4e58f5d
--- /dev/null
+++ b/bin/impala-virtualenv
@@ -0,0 +1,21 @@
+#!/bin/bash
+
+# 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.
+
+source "$(dirname "$0")/impala-python-common.sh"
+exec "$PY_ENV_DIR/bin/python" "$PY_ENV_DIR/bin/virtualenv" "$@"
diff --git a/shell/CMakeLists.txt b/shell/CMakeLists.txt
new file mode 100644
index 000000000..2ac96a4ca
--- /dev/null
+++ b/shell/CMakeLists.txt
@@ -0,0 +1,45 @@
+# 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.
+
+add_custom_target(shell_tarball DEPENDS gen-deps
+  COMMAND "${CMAKE_SOURCE_DIR}/shell/make_shell_tarball.sh"
+)
+
+add_custom_target(shell_pypi_package DEPENDS shell_tarball
+  COMMAND "${CMAKE_SOURCE_DIR}/shell/packaging/make_python_package.sh"
+)
+
+# A separate package target is needed because without OFFICIAL the file name is
+# non-deterministic. Uses a custom target to synchronize for multiple dependents.
+set(SHELL_TEST_PKG
+  "${CMAKE_SOURCE_DIR}/shell/build/dist/impala_shell-install-test.tar.gz")
+get_filename_component(SHELL_TEST_PKG_DIR "${SHELL_TEST_PKG}" DIRECTORY)
+# Generates SHELL_TEST_PKG
+add_custom_target(shell_pypi_test_package DEPENDS shell_tarball
+  COMMAND env BUILD_VERSION=install-test OFFICIAL=true DIST_DIR="${SHELL_TEST_PKG_DIR}"
+    "${CMAKE_SOURCE_DIR}/shell/packaging/make_python_package.sh"
+)
+
+set(PYTHON2_VENV "${CMAKE_SOURCE_DIR}/shell/build/py2_venv")
+
+add_custom_command(OUTPUT "${PYTHON2_VENV}" DEPENDS impala_python
+  COMMAND impala-virtualenv --python python2 "${PYTHON2_VENV}"
+)
+
+add_custom_target(shell_python2_install ALL DEPENDS "${PYTHON2_VENV}" shell_pypi_test_package
+  COMMAND "${PYTHON2_VENV}/bin/pip" install "${SHELL_TEST_PKG}"
+)
diff --git a/tests/custom_cluster/test_client_ssl.py b/tests/custom_cluster/test_client_ssl.py
index 4add40501..0c56948fd 100644
--- a/tests/custom_cluster/test_client_ssl.py
+++ b/tests/custom_cluster/test_client_ssl.py
@@ -32,7 +32,7 @@ from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
 from tests.common.impala_service import ImpaladService
 from tests.common.test_dimensions import create_client_protocol_dimension
 from tests.shell.util import run_impala_shell_cmd, run_impala_shell_cmd_no_expect, \
-    ImpalaShell
+    ImpalaShell, create_impala_shell_executable_dimension
 
 REQUIRED_MIN_OPENSSL_VERSION = 0x10001000L
 # Python supports TLSv1.2 from 2.7.9 officially but on Red Hat/CentOS Python2.7.5
@@ -142,6 +142,7 @@ class TestClientSsl(CustomClusterTestSuite):
   def add_test_dimensions(cls):
     super(TestClientSsl, cls).add_test_dimensions()
     cls.ImpalaTestMatrix.add_dimension(create_client_protocol_dimension())
+    cls.ImpalaTestMatrix.add_dimension(create_impala_shell_executable_dimension())
 
   @pytest.mark.execute_serially
   @CustomClusterTestSuite.with_args(impalad_args=WEBSERVER_SSL_ARGS,
diff --git a/tests/query_test/test_date_queries.py b/tests/query_test/test_date_queries.py
index a2cc1126c..34622f263 100644
--- a/tests/query_test/test_date_queries.py
+++ b/tests/query_test/test_date_queries.py
@@ -24,7 +24,7 @@ from tests.common.skip import (SkipIfS3, SkipIfABFS, SkipIfADLS, SkipIfLocal, Sk
                                SkipIfCOS)
 from tests.common.test_dimensions import (create_exec_option_dimension_from_dict,
     create_client_protocol_dimension, hs2_parquet_constraint)
-from tests.shell.util import ImpalaShell
+from tests.shell.util import create_impala_shell_executable_dimension
 
 
 class TestDateQueries(ImpalaTestSuite):
@@ -50,6 +50,7 @@ class TestDateQueries(ImpalaTestSuite):
     # via both protocols.
     cls.ImpalaTestMatrix.add_dimension(create_client_protocol_dimension())
     cls.ImpalaTestMatrix.add_constraint(hs2_parquet_constraint)
+    cls.ImpalaTestMatrix.add_dimension(create_impala_shell_executable_dimension())
 
   def test_queries(self, vector):
     if vector.get_value('table_format').file_format == 'avro':
diff --git a/tests/shell/test_shell_commandline.py b/tests/shell/test_shell_commandline.py
index 7f146855e..abfec1be5 100644
--- a/tests/shell/test_shell_commandline.py
+++ b/tests/shell/test_shell_commandline.py
@@ -39,8 +39,8 @@ from tests.common.test_dimensions import (
   create_uncompressed_text_dimension, create_single_exec_option_dimension)
 from time import sleep, time
 from util import (get_impalad_host_port, assert_var_substitution, run_impala_shell_cmd,
-                  ImpalaShell, IMPALA_SHELL_EXECUTABLE, SHELL_IS_PYTHON_2,
-                  build_shell_env, wait_for_query_state)
+                  ImpalaShell, build_shell_env, wait_for_query_state,
+                  create_impala_shell_executable_dimension, get_impala_shell_executable)
 from contextlib import closing
 
 
@@ -144,7 +144,9 @@ class TestImpalaShell(ImpalaTestSuite):
     cls.ImpalaTestMatrix.add_dimension(create_client_protocol_dimension())
     cls.ImpalaTestMatrix.add_dimension(create_client_protocol_strict_dimension())
     cls.ImpalaTestMatrix.add_constraint(lambda v:
-          v.get_value('protocol') != 'beeswax' or not v.get_value('strict_hs2_protocol'))
+        v.get_value('protocol') != 'beeswax' or not v.get_value('strict_hs2_protocol'))
+    # Test with python2 and the raw tarball
+    cls.ImpalaTestMatrix.add_dimension(create_impala_shell_executable_dimension())
 
   def test_no_args(self, vector):
     args = ['-q', DEFAULT_QUERY]
@@ -867,7 +869,8 @@ class TestImpalaShell(ImpalaTestSuite):
     # Building an one-off shell command instead of using Util::ImpalaShell since we need
     # to customize the impala daemon socket.
     protocol = vector.get_value("protocol")
-    shell_cmd = [IMPALA_SHELL_EXECUTABLE, "--protocol={0}".format(protocol)]
+    impala_shell_executable = get_impala_shell_executable(vector)
+    shell_cmd = [impala_shell_executable, "--protocol={0}".format(protocol)]
     if protocol == 'beeswax':
       expected_output = "get_default_configuration"
     else:
@@ -1103,16 +1106,13 @@ class TestImpalaShell(ImpalaTestSuite):
     assert "1\t1\t10.1" in result.stdout, result.stdout
     assert "2\t2\t20.2" in result.stdout, result.stdout
 
-    if (vector.get_value("protocol") in ('hs2', 'hs2-http')) and not SHELL_IS_PYTHON_2:
-      # The HS2 client returns binary values for float/double types, and these must
-      # be converted to strings for display. However, due to differences between the
-      # way that python2 and python3 represent floating point values, the output
-      # from the shell will differ with regard to which version of python the
-      # shell is running under.
-      assert "3\t3\t30.299999999999997" in result.stdout, result.stdout
-    else:
-      # python 2, or python 3 with beeswax protocol
-      assert "3\t3\t30.3" in result.stdout, result.stdout
+    # The HS2 client returns binary values for float/double types, and these must
+    # be converted to strings for display. However, due to differences between the
+    # way that python2 and python3 represent floating point values, the output
+    # from the shell will differ with regard to which version of python the
+    # shell is running under.
+    assert("3\t3\t30.299999999999997" in result.stdout or
+      "3\t3\t30.3" in result.stdout), result.stdout
 
     assert "4\t4\t40.4" in result.stdout, result.stdout
 
diff --git a/tests/shell/test_shell_interactive.py b/tests/shell/test_shell_interactive.py
index 300b042e6..143e3b4a1 100755
--- a/tests/shell/test_shell_interactive.py
+++ b/tests/shell/test_shell_interactive.py
@@ -43,9 +43,9 @@ from tests.common.skip import SkipIfLocal
 from tests.common.test_dimensions import (
   create_client_protocol_dimension, create_client_protocol_strict_dimension,
   create_uncompressed_text_dimension, create_single_exec_option_dimension)
-from tests.shell.util import get_unused_port
 from util import (assert_var_substitution, ImpalaShell, get_impalad_port, get_shell_cmd,
-                  get_open_sessions_metric, IMPALA_SHELL_EXECUTABLE, spawn_shell)
+                  get_open_sessions_metric, spawn_shell, get_unused_port,
+                  create_impala_shell_executable_dimension, get_impala_shell_executable)
 import SimpleHTTPServer
 import SocketServer
 
@@ -175,7 +175,9 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
     cls.ImpalaTestMatrix.add_dimension(create_client_protocol_dimension())
     cls.ImpalaTestMatrix.add_dimension(create_client_protocol_strict_dimension())
     cls.ImpalaTestMatrix.add_constraint(lambda v:
-          v.get_value('protocol') != 'beeswax' or not v.get_value('strict_hs2_protocol'))
+        v.get_value('protocol') != 'beeswax' or not v.get_value('strict_hs2_protocol'))
+    # Test with python2 and the raw tarball
+    cls.ImpalaTestMatrix.add_dimension(create_impala_shell_executable_dimension())
 
   def _expect_with_cmd(self, proc, cmd, vector, expectations=(), db="default"):
     """Executes a command on the expect process instance and verifies a set of
@@ -899,7 +901,7 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
   def test_line_with_leading_comment(self, vector, unique_database):
     # IMPALA-2195: A line with a comment produces incorrect command.
     if vector.get_value('strict_hs2_protocol'):
-      pytest.skip("Leading omments not supported in strict hs2 mode.")
+      pytest.skip("Leading comments not supported in strict hs2 mode.")
     table = "{0}.leading_comment".format(unique_database)
     run_impala_shell_interactive(vector, 'create table {0} (i int);'.format(table))
     result = run_impala_shell_interactive(vector, '-- comment\n'
@@ -984,30 +986,6 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
   def test_fix_infinite_loop(self, vector):
     # IMPALA-6337: Fix infinite loop.
 
-    # In case of TL;DR:
-    # - see IMPALA-9362 for details
-    # - see tests/shell/util.py for explanation of IMPALA_SHELL_EXECUTABLE
-    if os.getenv("IMPALA_HOME") not in IMPALA_SHELL_EXECUTABLE:
-      # The fix for IMPALA-6337 involved patching our internal verison of
-      # sqlparse 0.1.19 in ${IMPALA_HOME}/shell/ext-py. However, when we
-      # create the the stand-alone python package of the impala-shell for PyPI,
-      # we don't include the bundled 3rd party libs -- we expect users to
-      # install 3rd upstream libraries from PyPI.
-      #
-      # We could try to bundle sqlparse with the PyPI package, but there we
-      # run into the issue that the our bundled version is not python 3
-      # compatible. The real fix for this would be to upgrade to sqlparse 0.3.0,
-      # but that's not without complications. See IMPALA-9362 for details.
-      #
-      # For the time being, what this means is that IMPALA-6337 is fixed for
-      # people who are running the shell locally from any host/node that's part
-      # of a cluster where Impala is installed, but if they are running a
-      # standalone version of the shell on a client outside of a cluster, then
-      # they will still be relying on the upstream version of sqlparse 0.1.19,
-      # and so they may still be affected by the IMPALA-6337.
-      #
-      pytest.skip("Test will fail if shell is not part of dev environment.")
-
     result = run_impala_shell_interactive(vector, "select 1 + 1; \"\n;\";")
     if vector.get_value('strict_hs2_protocol'):
       assert '| 2   |' in result.stdout
@@ -1194,9 +1172,10 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
       pytest.skip()
 
     # Check that we get a message about the 503 error when we try to connect.
+    impala_shell_executable = get_impala_shell_executable(vector)
     shell_args = ["--protocol={0}".format(protocol),
                   "-i{0}:{1}".format(http_503_server.HOST, http_503_server.PORT)]
-    shell_proc = spawn_shell([IMPALA_SHELL_EXECUTABLE] + shell_args)
+    shell_proc = spawn_shell([impala_shell_executable] + shell_args)
     shell_proc.expect("HTTP code 503", timeout=10)
 
   def test_http_interactions_extra(self, vector, http_503_server_extra):
@@ -1208,10 +1187,11 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
       pytest.skip()
 
     # Check that we get a message about the 503 error when we try to connect.
+    impala_shell_executable = get_impala_shell_executable(vector)
     shell_args = ["--protocol={0}".format(protocol),
                   "-i{0}:{1}".format(http_503_server_extra.HOST,
                                      http_503_server_extra.PORT)]
-    shell_proc = spawn_shell([IMPALA_SHELL_EXECUTABLE] + shell_args)
+    shell_proc = spawn_shell([impala_shell_executable] + shell_args)
     shell_proc.expect("HTTP code 503: Service Unavailable \[EXTRA\]", timeout=10)
 
 
diff --git a/tests/shell/util.py b/tests/shell/util.py
index fc0365f3b..7a4ac21dc 100755
--- a/tests/shell/util.py
+++ b/tests/shell/util.py
@@ -37,6 +37,7 @@ from tests.common.impala_service import ImpaladService
 from tests.common.impala_test_suite import (IMPALAD_BEESWAX_HOST_PORT,
     IMPALAD_HS2_HOST_PORT, IMPALAD_HS2_HTTP_HOST_PORT,
     STRICT_HS2_HOST_PORT, STRICT_HS2_HTTP_HOST_PORT)
+from tests.common.test_vector import ImpalaTestDimension
 
 LOG = logging.getLogger('tests/shell/util.py')
 LOG.addHandler(logging.StreamHandler())
@@ -44,22 +45,6 @@ LOG.addHandler(logging.StreamHandler())
 SHELL_HISTORY_FILE = os.path.expanduser("~/.impalahistory")
 IMPALA_HOME = os.environ['IMPALA_HOME']
 
-# Note that pytest.config.getoption is deprecated usage. We use this
-# in a couple of other places. Ultimately, it needs to be addressed if
-# we ever want to get off of pytest 2.9.2.
-IMPALA_SHELL_EXECUTABLE = pytest.config.getoption('shell_executable')
-
-if IMPALA_SHELL_EXECUTABLE is None:
-  if ImpalaTestClusterProperties.get_instance().is_remote_cluster():
-    # With remote cluster testing, we cannot assume that the shell was built locally.
-    IMPALA_SHELL_EXECUTABLE = os.path.join(IMPALA_HOME, "bin/impala-shell.sh")
-  else:
-    # Test the locally built shell distribution.
-    IMPALA_SHELL_EXECUTABLE = os.path.join(
-        IMPALA_HOME, "shell/build", "impala-shell-" + IMPALA_LOCAL_BUILD_VERSION,
-        "impala-shell")
-
-
 def build_shell_env(env=None):
   """ Construct the environment for the shell to run in based on 'env', or the current
   process's environment if env is None."""
@@ -74,41 +59,6 @@ def build_shell_env(env=None):
   return env
 
 
-def get_python_version_for_shell_env():
-  """
-  Return the version of python belonging to the tested IMPALA_SHELL_EXECUTABLE.
-
-  We need this because some tests behave differently based on the version of
-  python being used to execute the impala-shell. However, since the test
-  framework itself is still being run with python2.7.x, sys.version_info
-  alone can't help us to determine the python version for the environment of
-  the shell executable. Instead, we have to invoke the shell, and then parse
-  the python version from the output. This information is present even in the
-  case of a fatal shell exception, e.g., not being unable to establish a
-  connection to an impalad.
-  """
-  version_check = Popen([IMPALA_SHELL_EXECUTABLE, '-q', 'version()'],
-                        stdout=PIPE, stderr=PIPE, env=build_shell_env())
-  stdout, stderr = version_check.communicate()
-
-  # e.g. Starting Impala with Kerberos authentication using Python 3.7.6
-  start_msg_line = stderr.split('\n')[0]
-  py_version = start_msg_line.split()[-1]   # e.g. 3.7.6
-  try:
-    major_version, minor_version, micro_version = py_version.split('.')
-    ret_val = int(major_version)
-  except (ValueError, UnboundLocalError) as e:
-    LOG.error(stderr)
-    sys.exit("Could not determine python version in shell env: {}".format(str(e)))
-
-  return ret_val
-
-
-# Since both test_shell_commandline and test_shell_interactive import from
-# this file, this check will be forced before any tests are run.
-SHELL_IS_PYTHON_2 = True if (get_python_version_for_shell_env() == 2) else False
-
-
 def assert_var_substitution(result):
   assert_pattern(r'\bfoo_number=.*$', 'foo_number= 123123', result.stdout, \
     'Numeric values not replaced correctly')
@@ -222,15 +172,16 @@ def get_impalad_port(vector):
 def get_shell_cmd(vector):
   """Get the basic shell command to start the shell, given the provided test vector.
   Returns the command as a list of string arguments."""
+  impala_shell_executable = get_impala_shell_executable(vector)
   if vector.get_value_with_default("strict_hs2_protocol", False):
     protocol = vector.get_value("protocol")
-    return [IMPALA_SHELL_EXECUTABLE,
+    return [impala_shell_executable,
             "--protocol={0}".format(protocol),
             "--strict_hs2_protocol",
             "--use_ldap_test_password",
             "-i{0}".format(get_impalad_host_port(vector))]
   else:
-    return [IMPALA_SHELL_EXECUTABLE,
+    return [impala_shell_executable,
             "--protocol={0}".format(vector.get_value("protocol")),
             "-i{0}".format(get_impalad_host_port(vector))]
 
@@ -257,7 +208,7 @@ class ImpalaShellResult(object):
 
 
 class ImpalaShell(object):
-  """A single instance of the Impala shell. The proces is started when this object is
+  """A single instance of the Impala shell. The process is started when this object is
      constructed, and then users should repeatedly call send_cmd(), followed eventually by
      get_result() to retrieve the process output. This constructor will wait until
      Impala shell is connected for the specified timeout unless wait_until_connected is
@@ -349,3 +300,40 @@ def wait_for_query_state(vector, stmt, state, max_retry=15):
     retry_count += 1
     time.sleep(1.0)
   raise Exception("Query didn't reach desired state: " + state)
+
+
+# Returns shell executable, and whether to include pypi variants
+def get_dev_impala_shell_executable():
+  # Note that pytest.config.getoption is deprecated usage. We use this
+  # in a couple of other places. Ultimately, it needs to be addressed if
+  # we ever want to get off of pytest 2.9.2.
+  impala_shell_executable = pytest.config.getoption('shell_executable')
+
+  if impala_shell_executable is not None:
+    return impala_shell_executable, False
+
+  if ImpalaTestClusterProperties.get_instance().is_remote_cluster():
+    # With remote cluster testing, we cannot assume that the shell was built locally.
+    return os.path.join(IMPALA_HOME, "bin/impala-shell.sh"), False
+  else:
+    # Test the locally built shell distribution.
+    return os.path.join(IMPALA_HOME, "shell/build",
+        "impala-shell-" + IMPALA_LOCAL_BUILD_VERSION, "impala-shell"), True
+
+
+def create_impala_shell_executable_dimension():
+  _, include_pypi = get_dev_impala_shell_executable()
+  if include_pypi:
+    return ImpalaTestDimension('impala_shell', 'dev', 'python2')
+  else:
+    return ImpalaTestDimension('impala_shell', 'dev')
+
+
+def get_impala_shell_executable(vector):
+  # impala-shell is invoked some places where adding a test vector may not make sense;
+  # use 'dev' as the default.
+  impala_shell_executable, _ = get_dev_impala_shell_executable()
+  return {
+    'dev': impala_shell_executable,
+    'python2': os.path.join(IMPALA_HOME, 'shell/build/py2_venv/bin/impala-shell')
+  }[vector.get_value_with_default('impala_shell', 'dev')]


[impala] 05/05: IMPALA-11343: Override isOpen to avoid thrift bug

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

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

commit 1285fc95ad237f18ff46f5c1d259e004058bbd6e
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Wed Jun 8 15:11:27 2022 -0700

    IMPALA-11343: Override isOpen to avoid thrift bug
    
    Overrides isOpen on TSSLSocket to work around THRIFT-5595.
    
    Change-Id: Iff9dfb3aaf578a7dbcedd3ca8e1478a5ecb958c3
    Reviewed-on: http://gerrit.cloudera.org:8080/18603
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Joe McDonnell <jo...@cloudera.com>
---
 shell/TSSLSocketWithWildcardSAN.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/shell/TSSLSocketWithWildcardSAN.py b/shell/TSSLSocketWithWildcardSAN.py
index 8f5b53ee0..95998711e 100755
--- a/shell/TSSLSocketWithWildcardSAN.py
+++ b/shell/TSSLSocketWithWildcardSAN.py
@@ -55,6 +55,10 @@ class TSSLSocketWithWildcardSAN(TSSLSocket.TSSLSocket):
                                    ca_certs=ca_certs, unix_socket=unix_socket,
                                    ssl_version=ssl.PROTOCOL_SSLv23)
 
+  # THRIFT-5595: override TSocket.isOpen because it's broken for TSSLSocket
+  def isOpen(self):
+    return self.handle is not None
+
   def _validate_cert(self):
     cert = self.handle.getpeercert()
     self.peercert = cert


[impala] 01/05: IMPALA-11302, IMPALA-11303: Fix error handling of external Iceberg tables

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

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

commit cee086f685e1b641da14513cc24ad11c56dee7fd
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Tue May 24 14:14:55 2022 +0200

    IMPALA-11302, IMPALA-11303: Fix error handling of external Iceberg tables
    
    IMPALA-11302: Improve error message for CREATE EXTERNAL TABLE iceberg
    command
    
    The error message complained about failed table loading. The new
    error message is more precise, and also notifies the user to use
    plain 'CREATE TABLE' for creating new Iceberg tables.
    
    IMPALA-11303: Exception is not raised for Iceberg DDL that misses
    LOCATION clause
    
    We returned the error in the result summary. Instead of that now
    we raise an error, and also notify  the user about how to create
    new Iceberg tables.
    
    Testing:
     * e2e tests
    
    Change-Id: I659115cc97a1a00e1ddf3fbb7dbe1f286bf1edcf
    Reviewed-on: http://gerrit.cloudera.org:8080/18563
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../apache/impala/analysis/CreateTableStmt.java    | 29 ++++++++++++++++++++--
 .../queries/QueryTest/iceberg-create.test          |  9 -------
 .../queries/QueryTest/iceberg-negative.test        | 17 +++++++++++++
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
index 9976b11b9..114bbdb04 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
@@ -340,8 +340,7 @@ public class CreateTableStmt extends StatementBase {
     }
 
     analyzeKuduTableProperties(analyzer);
-    if (isExternal() && !Boolean.parseBoolean(getTblProperties().get(
-        Table.TBL_PROP_EXTERNAL_TABLE_PURGE))) {
+    if (isExternalWithNoPurge()) {
       // this is an external table
       analyzeExternalKuduTableParams(analyzer);
     } else {
@@ -736,6 +735,19 @@ public class CreateTableStmt extends StatementBase {
       throw new AnalysisException(String.format("%s cannot be set for Iceberg table " +
           "stored in hive.catalog", IcebergTable.ICEBERG_CATALOG_LOCATION));
     }
+    if (isExternalWithNoPurge()) {
+      String tableId = getTblProperties().get(IcebergTable.ICEBERG_TABLE_IDENTIFIER);
+      if (tableId == null || tableId.isEmpty()) {
+        tableId = getTblProperties().get(Catalogs.NAME);
+      }
+      if (tableId == null || tableId.isEmpty()) {
+        throw new AnalysisException(String.format("Table property '%s' is necessary " +
+            "for external Iceberg tables stored in hive.catalog. " +
+            "For creating a completely new Iceberg table, use 'CREATE TABLE' " +
+            "(no EXTERNAL keyword).",
+            IcebergTable.ICEBERG_TABLE_IDENTIFIER));
+      }
+    }
   }
 
   private void validateTableInHadoopCatalog() throws AnalysisException {
@@ -757,6 +769,11 @@ public class CreateTableStmt extends StatementBase {
       throw new AnalysisException(String.format("%s cannot be set for Iceberg table " +
           "stored in hadoop.tables", IcebergTable.ICEBERG_CATALOG_LOCATION));
     }
+    if (isExternalWithNoPurge() && getLocation() == null) {
+      throw new AnalysisException("Set LOCATION for external Iceberg tables " +
+          "stored in hadoop.tables. For creating a completely new Iceberg table, use " +
+          "'CREATE TABLE' (no EXTERNAL keyword).");
+    }
   }
 
   private void validateTableInCatalogs() {
@@ -829,4 +846,12 @@ public class CreateTableStmt extends StatementBase {
     getColumnDefs().addAll(getPartitionColumnDefs());
     getPartitionColumnDefs().clear();
   }
+
+  /**
+   * @return true for external tables that don't have "external.table.purge" set to true.
+   */
+  private boolean isExternalWithNoPurge() {
+    return isExternal() && !Boolean.parseBoolean(getTblProperties().get(
+      Table.TBL_PROP_EXTERNAL_TABLE_PURGE));
+  }
 }
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
index 4ba9abd3e..9cf268530 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
@@ -66,15 +66,6 @@ DESCRIBE iceberg_hadoop_tables;
 STRING,STRING,STRING,STRING
 ====
 ---- QUERY
-CREATE EXTERNAL TABLE iceberg_hadoop_tbls_external(
-  level STRING
-)
-STORED AS ICEBERG
-TBLPROPERTIES('iceberg.catalog'='hadoop.tables');
----- RESULTS
-'Location is necessary for external iceberg table.'
-====
----- QUERY
 CREATE TABLE iceberg_hadoop_tbls_with_loc(
   level STRING
 )
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
index 0a3555f51..15dc723e3 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
@@ -79,6 +79,23 @@ TBLPROPERTIES('iceberg.catalog'='hadoop.catalog',
 AnalysisException: Table property 'iceberg.catalog_location' is necessary for Iceberg table with 'hadoop.catalog'.
 ====
 ---- QUERY
+CREATE EXTERNAL TABLE iceberg_hadoop_tbls_external(
+  level STRING
+)
+STORED AS ICEBERG
+TBLPROPERTIES('iceberg.catalog'='hadoop.tables');
+---- CATCH
+Set LOCATION for external Iceberg tables stored in hadoop.tables. For creating a completely new Iceberg table, use 'CREATE TABLE' (no EXTERNAL keyword).
+====
+---- QUERY
+CREATE EXTERNAL TABLE iceberg_hive_tbls_external(
+  level STRING
+)
+STORED AS ICEBERG;
+---- CATCH
+Table property 'iceberg.table_identifier' is necessary for external Iceberg tables stored in hive.catalog. For creating a completely new Iceberg table, use 'CREATE TABLE' (no EXTERNAL keyword).
+====
+---- QUERY
 CREATE EXTERNAL TABLE fake_iceberg_table_hadoop_catalog
 STORED AS ICEBERG
 TBLPROPERTIES('iceberg.catalog'='hadoop.catalog',


[impala] 02/05: IMPALA-1682: Support printing the output of a query (rows) vertically.

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

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

commit c7784bde55d86a25388f5a61e64cda47a3ccaeee
Author: yx91490 <yx...@126.com>
AuthorDate: Tue Mar 29 14:19:21 2022 +0800

    IMPALA-1682: Support printing the output of a query (rows) vertically.
    
    In vertical mode, impala-shell will print each row in the format:
    firstly print a line contains line number, then print this row's columns
    line by line, each column line started with it's name and a colon.
    
    To enable it: use shell option '-E' or '--vertical', or 'set VERTICAL=
    true' in interactive mode. to disable it in interactive mode: 'set
    VERTICAL=false'. NOTICE: it will be disabled if '-B' option or 'set
    WRITE_DELIMITED=true' is specified.
    
    Tests:
    add methods in test_shell_interactive.py and test_shell_commandline.py.
    
    Change-Id: I5cee48d5a239d6b7c0f51331275524a25130fadf
    Reviewed-on: http://gerrit.cloudera.org:8080/18549
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 docs/topics/impala_shell_options.xml  | 16 ++++++++++++++++
 shell/impala_shell.py                 |  7 ++++++-
 shell/impala_shell_config_defaults.py |  1 +
 shell/option_parser.py                |  4 ++++
 shell/shell_output.py                 | 36 ++++++++++++++++++++++++++++-------
 tests/shell/test_shell_commandline.py | 18 +++++++++++++-----
 tests/shell/test_shell_interactive.py | 13 +++++++++++++
 7 files changed, 82 insertions(+), 13 deletions(-)

diff --git a/docs/topics/impala_shell_options.xml b/docs/topics/impala_shell_options.xml
index 87496f465..d6660832c 100644
--- a/docs/topics/impala_shell_options.xml
+++ b/docs/topics/impala_shell_options.xml
@@ -188,6 +188,22 @@ under the License.
                 </p>
               </entry>
             </row>
+            <row>
+              <entry>
+                <p> -E or </p>
+                <p>--vertical </p>
+              </entry>
+              <entry rev="2.0.0">
+                <p>
+                  vertical=true
+                </p>
+              </entry>
+              <entry>
+                <p>
+                  Print the output of a query (rows) vertically, no effects if <codeph>-B</codeph> is specified. Added in Impala 4.2.
+                </p>
+              </entry>
+            </row>
             <row>
               <entry>
                 <p> -p or </p>
diff --git a/shell/impala_shell.py b/shell/impala_shell.py
index 6d7b1e59d..735f2354a 100755
--- a/shell/impala_shell.py
+++ b/shell/impala_shell.py
@@ -44,7 +44,7 @@ from impala_client import ImpalaHS2Client, StrictHS2Client, \
 from impala_shell_config_defaults import impala_shell_defaults
 from option_parser import get_option_parser, get_config_from_file
 from shell_output import (DelimitedOutputFormatter, OutputStream, PrettyOutputFormatter,
-                          OverwritingStdErrOutputStream)
+                          OverwritingStdErrOutputStream, VerticalOutputFormatter)
 from subprocess import call
 from shell_exceptions import (RPCException, DisconnectedException, QueryStateException,
     QueryCancelledByShellException, MissingThriftMethodException)
@@ -167,6 +167,7 @@ class ImpalaShell(cmd.Cmd, object):
     'VERBOSE' : (lambda x: x in ImpalaShell.TRUE_STRINGS, "verbose"),
     'DELIMITER' : (lambda x: " " if x == '\\s' else x, "output_delimiter"),
     'OUTPUT_FILE' : (lambda x: None if x == '' else x, "output_file"),
+    'VERTICAL': (lambda x: x in ImpalaShell.TRUE_STRINGS, "vertical"),
   }
 
   # Minimum time in seconds between two calls to get the exec summary.
@@ -219,6 +220,7 @@ class ImpalaShell(cmd.Cmd, object):
         else options.output_delimiter
     self.write_delimited = options.write_delimited
     self.print_header = options.print_header
+    self.vertical = options.vertical
 
     self.progress_stream = OverwritingStdErrOutputStream()
 
@@ -1200,6 +1202,9 @@ class ImpalaShell(cmd.Cmd, object):
       # print the column names
       if self.print_header:
         self.output_stream.write([column_names])
+    elif self.vertical:
+      formatter = VerticalOutputFormatter(column_names)
+      self.output_stream = OutputStream(formatter, filename=self.output_file)
     else:
       prettytable = self.construct_table_with_header(column_names)
       formatter = PrettyOutputFormatter(prettytable)
diff --git a/shell/impala_shell_config_defaults.py b/shell/impala_shell_config_defaults.py
index dbaa2b62b..513a16af7 100644
--- a/shell/impala_shell_config_defaults.py
+++ b/shell/impala_shell_config_defaults.py
@@ -41,6 +41,7 @@ impala_shell_defaults = {
             'output_delimiter': '\\t',
             'output_file': None,
             'print_header': False,
+            'vertical': False,
             'live_progress': True,  # The option only applies to interactive shell session
             'live_summary': False,
             'query': None,
diff --git a/shell/option_parser.py b/shell/option_parser.py
index d5274d3b8..beee7f412 100755
--- a/shell/option_parser.py
+++ b/shell/option_parser.py
@@ -191,6 +191,10 @@ def get_option_parser(defaults):
                     action="store_true",
                     help="Print column names in delimited mode"
                          " when pretty-printed.")
+  parser.add_option("-E", "--vertical",
+                    dest="vertical",
+                    action="store_true",
+                    help="Print the output of a query (rows) vertically.")
   parser.add_option("--output_delimiter", dest="output_delimiter",
                     help="Field delimiter to use for output in delimited mode")
   parser.add_option("-s", "--kerberos_service_name",
diff --git a/shell/shell_output.py b/shell/shell_output.py
index 608a8ca92..371cb911d 100644
--- a/shell/shell_output.py
+++ b/shell/shell_output.py
@@ -70,7 +70,8 @@ class DelimitedOutputFormatter(object):
         field_delim_bytes = bytearray(field_delim, 'utf-8')
         self.field_delim = field_delim_bytes.decode('unicode_escape')
       else:
-        self.field_delim = field_delim.decode('unicode_escape')
+        # csv.writer in python2 requires an ascii string delimiter
+        self.field_delim = field_delim.decode('unicode_escape').encode('ascii', 'ignore')
       # IMPALA-8652, the delimiter should be a 1-character string and verified already
       assert len(self.field_delim) == 1
 
@@ -78,12 +79,7 @@ class DelimitedOutputFormatter(object):
     """Returns string containing UTF-8-encoded representation of the table data."""
     # csv.writer expects a file handle to the input.
     temp_buffer = StringIO()
-    if sys.version_info.major == 2:
-      # csv.writer in python2 requires an ascii string delimiter
-      delim = self.field_delim.encode('ascii', 'ignore')
-    else:
-      delim = self.field_delim
-    writer = csv.writer(temp_buffer, delimiter=delim,
+    writer = csv.writer(temp_buffer, delimiter=self.field_delim,
                         lineterminator='\n', quoting=csv.QUOTE_MINIMAL)
     for row in rows:
       if sys.version_info.major == 2:
@@ -102,6 +98,32 @@ class DelimitedOutputFormatter(object):
     return rows
 
 
+class VerticalOutputFormatter(DelimitedOutputFormatter):
+  def __init__(self, column_names):
+    DelimitedOutputFormatter.__init__(self, field_delim="\n")
+    self.column_names = column_names
+    self.column_name_max_len = max([len(s) for s in column_names])
+
+  def format(self, rows):
+    """Returns string containing UTF-8-encoded representation of the table data."""
+    # csv.writer expects a file handle to the input.
+    temp_buffer = StringIO()
+    writer = csv.writer(temp_buffer, delimiter=self.field_delim,
+                        lineterminator='\n', quoting=csv.QUOTE_MINIMAL)
+    for r, row in enumerate(rows):
+      if sys.version_info.major == 2:
+        row = [val.encode('utf-8', 'replace') if isinstance(val, unicode) else val
+            for val in row]
+      writer.writerow(["************************************** " +
+        str(r + 1) + ".row **************************************"])
+      for c, val in enumerate(row):
+        row[c] = self.column_names[c].rjust(self.column_name_max_len) + ": " + val
+      writer.writerow(row)
+    rows = temp_buffer.getvalue().rstrip()
+    temp_buffer.close()
+    return rows
+
+
 class OutputStream(object):
   def __init__(self, formatter, filename=None):
     """Helper class for writing query output.
diff --git a/tests/shell/test_shell_commandline.py b/tests/shell/test_shell_commandline.py
index 6fda37a32..7f146855e 100644
--- a/tests/shell/test_shell_commandline.py
+++ b/tests/shell/test_shell_commandline.py
@@ -297,17 +297,25 @@ class TestImpalaShell(ImpalaTestSuite):
         result.stderr
 
   def test_output_format(self, vector):
-    expected_output = ['1'] * 3
-    args = ['-q', 'select 1,1,1', '-B', '--quiet']
-    result = run_impala_shell_cmd(vector, args)
+    expected_output = ['1', '2', '3']
+    args = ['-q', 'select 1 as col_00001, 2 as col_2, 3 as col_03', '--quiet']
+    result = run_impala_shell_cmd(vector, args + ['-B'])
     actual_output = [r.strip() for r in result.stdout.split('\t')]
     assert actual_output == expected_output
-    result = run_impala_shell_cmd(vector, args + ['--output_delim=|'])
+    result = run_impala_shell_cmd(vector, args + ['-B', '--output_delim=|'])
     actual_output = [r.strip() for r in result.stdout.split('|')]
     assert actual_output == expected_output
-    result = run_impala_shell_cmd(vector, args + ['--output_delim=||'],
+    result = run_impala_shell_cmd(vector, args + ['-B', '--output_delim=||'],
                                   expect_success=False)
     assert "Illegal delimiter" in result.stderr
+    result = run_impala_shell_cmd(vector, args + ['-E'])
+    result_rows = result.stdout.strip().split('\n')
+    assert len(result_rows) == 4
+    assert "************************************** " \
+      "1.row **************************************" == result_rows[0]
+    assert "col_00001: 1" == result_rows[1]
+    assert "    col_2: 2" == result_rows[2]
+    assert "   col_03: 3" == result_rows[3]
 
   def test_do_methods(self, vector, empty_table):
     """Ensure that the do_ methods in the shell work.
diff --git a/tests/shell/test_shell_interactive.py b/tests/shell/test_shell_interactive.py
index cd2db996b..300b042e6 100755
--- a/tests/shell/test_shell_interactive.py
+++ b/tests/shell/test_shell_interactive.py
@@ -254,6 +254,19 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
     result = p.get_result()
     assert "21,VIETNAM,2" in result.stdout
 
+  def test_vertical(self, vector):
+    """Test output rows in vertical mode"""
+    p = ImpalaShell(vector)
+    p.send_cmd("use tpch")
+    p.send_cmd("set vertical=true")
+    p.send_cmd("select N_NATIONKEY, N_NAME from nation limit 1")
+    result = p.get_result()
+    assert "+----------------+" not in result.stdout
+    assert "************************************** " \
+      "1.row **************************************" in result.stdout, result.stdout
+    assert "n_nationkey: " in result.stdout, result.stdout
+    assert "n_name: " in result.stdout, result.stdout
+
   @pytest.mark.execute_serially
   def test_print_to_file(self, vector):
     """Test print to output file and unset"""


[impala] 04/05: IMPALA-8373: Test impala-shell with python3

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

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

commit 181fd9406887d33eb97b8cfc6441f61d8defc8a9
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Thu Jun 2 16:02:06 2022 -0700

    IMPALA-8373: Test impala-shell with python3
    
    Sets up a python3 virtualenv, installs impala-shell into it, and runs
    tests.
    
    Change-Id: I8e123aecd53a7ded44a7da7eb8c8b853cebbfc56
    Reviewed-on: http://gerrit.cloudera.org:8080/18588
    Reviewed-by: Joe McDonnell <jo...@cloudera.com>
    Tested-by: Joe McDonnell <jo...@cloudera.com>
---
 CMakeLists.txt                        |  4 +++-
 bin/bootstrap_build.sh                |  2 +-
 bin/bootstrap_system.sh               |  4 ++--
 shell/CMakeLists.txt                  | 11 ++++++++++-
 tests/shell/test_shell_commandline.py |  2 +-
 tests/shell/test_shell_interactive.py |  2 +-
 tests/shell/util.py                   |  5 +++--
 7 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 30280b12e..346d6687d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -467,8 +467,10 @@ add_custom_target(impala_python ALL
   COMMAND "${CMAKE_SOURCE_DIR}/bin/init-impala-python.sh"
 )
 
+add_custom_target(impala_shell_pypi ALL DEPENDS shell_python2_install shell_python3_install)
+
 add_custom_target(notests_independent_targets DEPENDS
-  java cscope tarballs impala_python shell_python2_install
+  java cscope tarballs impala_python impala_shell_pypi
 )
 add_custom_target(notests_regular_targets DEPENDS
   impalad statestored catalogd admissiond fesupport loggingsupport ImpalaUdf udasample udfsample impala-profile-tool
diff --git a/bin/bootstrap_build.sh b/bin/bootstrap_build.sh
index a450ef7ea..3d2c24a04 100755
--- a/bin/bootstrap_build.sh
+++ b/bin/bootstrap_build.sh
@@ -32,7 +32,7 @@ set -euxo pipefail
 # Install non-java dependencies:
 sudo apt-get update
 sudo apt-get --yes install g++ gcc git libsasl2-dev libssl-dev make \
-    python-dev python-setuptools libffi-dev libkrb5-dev
+    python-dev python-setuptools python3-dev libffi-dev libkrb5-dev
 
 
 source /etc/lsb-release
diff --git a/bin/bootstrap_system.sh b/bin/bootstrap_system.sh
index ec7919f77..d5ea5398d 100755
--- a/bin/bootstrap_system.sh
+++ b/bin/bootstrap_system.sh
@@ -218,7 +218,7 @@ ubuntu apt-get update
 ubuntu apt-get --yes install ccache curl gawk g++ gcc libffi-dev \
         libkrb5-dev krb5-admin-server krb5-kdc krb5-user libsasl2-dev \
         libsasl2-modules libsasl2-modules-gssapi-mit libssl-dev make ninja-build \
-        python-dev python-setuptools postgresql ssh wget vim-common psmisc \
+        python-dev python-setuptools python3-dev postgresql ssh wget vim-common psmisc \
         lsof openjdk-8-jdk openjdk-8-source openjdk-8-dbg apt-utils git ant
 # Required by Kudu in the minicluster
 ubuntu20 apt-get --yes install libtinfo5
@@ -266,7 +266,7 @@ redhat sudo yum install -y curl gawk gcc gcc-c++ git krb5-devel krb5-server \
         postgresql postgresql-server \
         wget vim-common nscd cmake fuse-devel zlib-devel \
         psmisc lsof openssh-server redhat-lsb java-1.8.0-openjdk-devel \
-        java-1.8.0-openjdk-src
+        java-1.8.0-openjdk-src python3-devel
 
 # Enable the Powertools repo for snappy-devel on RedHat 8
 redhat8 sudo yum install -y dnf-plugins-core
diff --git a/shell/CMakeLists.txt b/shell/CMakeLists.txt
index 2ac96a4ca..e429f5d7c 100644
--- a/shell/CMakeLists.txt
+++ b/shell/CMakeLists.txt
@@ -35,11 +35,20 @@ add_custom_target(shell_pypi_test_package DEPENDS shell_tarball
 )
 
 set(PYTHON2_VENV "${CMAKE_SOURCE_DIR}/shell/build/py2_venv")
+set(PYTHON3_VENV "${CMAKE_SOURCE_DIR}/shell/build/py3_venv")
 
 add_custom_command(OUTPUT "${PYTHON2_VENV}" DEPENDS impala_python
   COMMAND impala-virtualenv --python python2 "${PYTHON2_VENV}"
 )
 
-add_custom_target(shell_python2_install ALL DEPENDS "${PYTHON2_VENV}" shell_pypi_test_package
+add_custom_target(shell_python2_install DEPENDS "${PYTHON2_VENV}" shell_pypi_test_package
   COMMAND "${PYTHON2_VENV}/bin/pip" install "${SHELL_TEST_PKG}"
 )
+
+add_custom_command(OUTPUT "${PYTHON3_VENV}" DEPENDS impala_python
+  COMMAND impala-virtualenv --python python3 "${PYTHON3_VENV}"
+)
+
+add_custom_target(shell_python3_install DEPENDS "${PYTHON3_VENV}" shell_pypi_test_package
+  COMMAND "${PYTHON3_VENV}/bin/pip" install "${SHELL_TEST_PKG}"
+)
diff --git a/tests/shell/test_shell_commandline.py b/tests/shell/test_shell_commandline.py
index abfec1be5..fbcbdbced 100644
--- a/tests/shell/test_shell_commandline.py
+++ b/tests/shell/test_shell_commandline.py
@@ -145,7 +145,7 @@ class TestImpalaShell(ImpalaTestSuite):
     cls.ImpalaTestMatrix.add_dimension(create_client_protocol_strict_dimension())
     cls.ImpalaTestMatrix.add_constraint(lambda v:
         v.get_value('protocol') != 'beeswax' or not v.get_value('strict_hs2_protocol'))
-    # Test with python2 and the raw tarball
+    # Test with python2, python3, and the raw tarball
     cls.ImpalaTestMatrix.add_dimension(create_impala_shell_executable_dimension())
 
   def test_no_args(self, vector):
diff --git a/tests/shell/test_shell_interactive.py b/tests/shell/test_shell_interactive.py
index 143e3b4a1..14e185420 100755
--- a/tests/shell/test_shell_interactive.py
+++ b/tests/shell/test_shell_interactive.py
@@ -176,7 +176,7 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
     cls.ImpalaTestMatrix.add_dimension(create_client_protocol_strict_dimension())
     cls.ImpalaTestMatrix.add_constraint(lambda v:
         v.get_value('protocol') != 'beeswax' or not v.get_value('strict_hs2_protocol'))
-    # Test with python2 and the raw tarball
+    # Test with python2, python3, and the raw tarball
     cls.ImpalaTestMatrix.add_dimension(create_impala_shell_executable_dimension())
 
   def _expect_with_cmd(self, proc, cmd, vector, expectations=(), db="default"):
diff --git a/tests/shell/util.py b/tests/shell/util.py
index 7a4ac21dc..faef13825 100755
--- a/tests/shell/util.py
+++ b/tests/shell/util.py
@@ -324,7 +324,7 @@ def get_dev_impala_shell_executable():
 def create_impala_shell_executable_dimension():
   _, include_pypi = get_dev_impala_shell_executable()
   if include_pypi:
-    return ImpalaTestDimension('impala_shell', 'dev', 'python2')
+    return ImpalaTestDimension('impala_shell', 'dev', 'python2', 'python3')
   else:
     return ImpalaTestDimension('impala_shell', 'dev')
 
@@ -335,5 +335,6 @@ def get_impala_shell_executable(vector):
   impala_shell_executable, _ = get_dev_impala_shell_executable()
   return {
     'dev': impala_shell_executable,
-    'python2': os.path.join(IMPALA_HOME, 'shell/build/py2_venv/bin/impala-shell')
+    'python2': os.path.join(IMPALA_HOME, 'shell/build/py2_venv/bin/impala-shell'),
+    'python3': os.path.join(IMPALA_HOME, 'shell/build/py3_venv/bin/impala-shell')
   }[vector.get_value_with_default('impala_shell', 'dev')]