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

[impala] branch master updated (decb46aa0 -> 13bbff4e4)

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

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


    from decb46aa0 IMPALA-9410: Support resolving ORC file columns by names
     new d84ce571b [tools] Fix tar argument ordering to exclude .pyc
     new a57c9e0b9 [tools] Fix building shell_pypi_package
     new 13bbff4e4 IMPALA-11323: Don't evaluate constants-only inferred predicates

The 3 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                                     |  2 +-
 .../main/java/org/apache/impala/analysis/Expr.java | 39 +++++++++++++++++-----
 .../java/org/apache/impala/planner/PlanNode.java   | 19 ++++++++---
 shell/make_shell_tarball.sh                        |  4 +--
 shell/packaging/make_python_package.sh             |  6 ++--
 .../queries/QueryTest/inline-view.test             | 16 ++++++++-
 6 files changed, 67 insertions(+), 19 deletions(-)


[impala] 01/03: [tools] Fix tar argument ordering to exclude .pyc

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

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

commit d84ce571b1f8e99e0448c6be9e32cb0438bd5555
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Thu Jun 2 14:53:39 2022 -0700

    [tools] Fix tar argument ordering to exclude .pyc
    
    Moves `--exclude='*.pyc'` before the files to add as required by tar on
    Debian. Fixes the message
    > tar: --exclude ‘*.pyc’ has no effect
    
    Change-Id: I79e344f2df6f27e8961ed9d949e59d44cce5d10f
    Reviewed-on: http://gerrit.cloudera.org:8080/18587
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 shell/make_shell_tarball.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/shell/make_shell_tarball.sh b/shell/make_shell_tarball.sh
index 05be30a63..395a14382 100755
--- a/shell/make_shell_tarball.sh
+++ b/shell/make_shell_tarball.sh
@@ -158,5 +158,5 @@ cp ${SHELL_HOME}/compatibility.py ${TARBALL_ROOT}
 
 pushd ${BUILD_DIR} > /dev/null
 echo "Making tarball in ${BUILD_DIR}"
-tar czf ${BUILD_DIR}/impala-shell-${VERSION}.tar.gz ./impala-shell-${VERSION}/\
-    --exclude="*.pyc" || popd 2>&1 > /dev/null
+tar czf ${BUILD_DIR}/impala-shell-${VERSION}.tar.gz --exclude="*.pyc" \
+    ./impala-shell-${VERSION}/ || popd 2>&1 > /dev/null


[impala] 03/03: IMPALA-11323: Don't evaluate constants-only inferred predicates

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

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

commit 13bbff4e4e5fc5d459cc6f7a5512f84ceba897cd
Author: Steve Carlin <sc...@cloudera.com>
AuthorDate: Wed Jun 1 12:29:56 2022 -0700

    IMPALA-11323: Don't evaluate constants-only inferred predicates
    
    IMPALA-10182 fixed the problem of creating inferred predicates when
    both sides of an equality predicate came from the same slot.
    
    Inferred predicates also should not be created when both sides
    of an equality predicate are constant values which do not have
    scan slots.
    
    Change-Id: If1cd4559dda406d2d38703ed594b70b41ed336fd
    Reviewed-on: http://gerrit.cloudera.org:8080/18579
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Aman Sinha <am...@cloudera.com>
---
 .../main/java/org/apache/impala/analysis/Expr.java | 39 +++++++++++++++++-----
 .../java/org/apache/impala/planner/PlanNode.java   | 19 ++++++++---
 .../queries/QueryTest/inline-view.test             | 16 ++++++++-
 3 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/analysis/Expr.java b/fe/src/main/java/org/apache/impala/analysis/Expr.java
index 76175d51c..0bc470053 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java
@@ -1642,24 +1642,47 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
   }
 
   /**
-   * Returns the descriptor of the scan slot that directly or indirectly produces
-   * the values of 'this' SlotRef. Traverses the source exprs of intermediate slot
-   * descriptors to resolve materialization points (e.g., aggregations).
-   * Returns null if 'e' or any source expr of 'e' is not a SlotRef or cast SlotRef.
-   */
-  public SlotDescriptor findSrcScanSlot() {
+   * Returns the source expression for this expression. Traverses the source
+   * exprs of intermediate slot descriptors to resolve materialization points
+   * (e.g., aggregations). Returns null if there are multiple source Exprs
+   * mapped to the expression at any given point.
+   */
+  public Expr findSrcExpr() {
+    // If the source expression is a constant expression, it won't have a scanSlotRef
+    // and we can return this.
+    if (isConstant()) {
+      return this;
+    }
     SlotRef slotRef = unwrapSlotRef(false);
     if (slotRef == null) return null;
     SlotDescriptor slotDesc = slotRef.getDesc();
-    if (slotDesc.isScanSlot()) return slotDesc;
+    if (slotDesc.isScanSlot()) return slotRef;
     if (slotDesc.getSourceExprs().size() == 1) {
-      return slotDesc.getSourceExprs().get(0).findSrcScanSlot();
+      return slotDesc.getSourceExprs().get(0).findSrcExpr();
     }
     // No known source expr, or there are several source exprs meaning the slot is
     // has no single source table.
     return null;
   }
 
+  /**
+   * Returns the descriptor of the scan slot that directly or indirectly produces
+   * the values of 'this' SlotRef. Traverses the source exprs of intermediate slot
+   * descriptors to resolve materialization points (e.g., aggregations).
+   * Returns null if 'e' or any source expr of 'e' is not a SlotRef or cast SlotRef.
+   */
+  public SlotDescriptor findSrcScanSlot() {
+    Expr sourceExpr = findSrcExpr();
+    if (sourceExpr == null) {
+      return null;
+    }
+    SlotRef slotRef = sourceExpr.unwrapSlotRef(false);
+    if (slotRef == null) {
+      return null;
+    }
+    return slotRef.getDesc();
+  }
+
   /**
    * Pushes negation to the individual operands of a predicate
    * tree rooted at 'root'.
diff --git a/fe/src/main/java/org/apache/impala/planner/PlanNode.java b/fe/src/main/java/org/apache/impala/planner/PlanNode.java
index b59acedc3..2dc21afe2 100644
--- a/fe/src/main/java/org/apache/impala/planner/PlanNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/PlanNode.java
@@ -574,12 +574,23 @@ abstract public class PlanNode extends TreeNode<PlanNode> {
     // Check if this is an inferred identity predicate i.e for c1 = c2 both
     // sides are pointing to the same source slot. In such cases it is wrong
     // to add the predicate to the SELECT node because it will incorrectly
-    // eliminate rows with NULL values.
+    // eliminate rows with NULL values. We also check if both sides are pointing
+    // to equal constant values.
     for (Expr e : conjuncts) {
       if (e instanceof BinaryPredicate && ((BinaryPredicate) e).isInferred()) {
-        SlotDescriptor lhs = ((BinaryPredicate) e).getChild(0).findSrcScanSlot();
-        SlotDescriptor rhs = ((BinaryPredicate) e).getChild(1).findSrcScanSlot();
-        if (lhs != null && rhs != null && lhs.equals(rhs)) continue;
+        Expr lhsSrcExpr = ((BinaryPredicate) e).getChild(0).findSrcExpr();
+        Expr rhsSrcExpr  = ((BinaryPredicate) e).getChild(1).findSrcExpr();
+        if (lhsSrcExpr != null && rhsSrcExpr != null) {
+          if (lhsSrcExpr.isConstant() && rhsSrcExpr.isConstant() &&
+              lhsSrcExpr.equals(rhsSrcExpr)) {
+            continue;
+          }
+          if (lhsSrcExpr instanceof SlotRef && rhsSrcExpr instanceof SlotRef) {
+            SlotRef lhsSlotRef = (SlotRef) lhsSrcExpr;
+            SlotRef rhsSlotRef = (SlotRef) rhsSrcExpr;
+            if (lhsSlotRef.getDesc().equals(rhsSlotRef.getDesc())) continue;
+          }
+        }
       }
       finalConjuncts.add(e);
     }
diff --git a/testdata/workloads/functional-query/queries/QueryTest/inline-view.test b/testdata/workloads/functional-query/queries/QueryTest/inline-view.test
index 2cbda7bba..5c0cc185d 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/inline-view.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/inline-view.test
@@ -578,4 +578,18 @@ NULL,NULL
 NULL,NULL
 ---- TYPES
 INT, INT
-====
\ No newline at end of file
+====
+---- QUERY
+# IMPALA-11323: Constant expressions should get filtered out so they
+# don't get placed in the select node in the planner.
+with t as (select 1 a), v as
+  (select distinct a, cast(null as smallint) b, cast(null as smallint) c from t)
+select distinct a,b,c from v
+  union all
+select distinct a,b,c from v;
+---- RESULTS
+1,NULL,NULL
+1,NULL,NULL
+---- TYPES
+TINYINT, SMALLINT, SMALLINT
+====


[impala] 02/03: [tools] Fix building shell_pypi_package

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

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

commit a57c9e0b9aa1ba624245d515b95aff11a22165e1
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Wed Jun 1 10:54:04 2022 -0700

    [tools] Fix building shell_pypi_package
    
    Fixes an error building shell_pypi_package with junit_xml_wrapper.sh.
    Previously `make shell_pypi_package` would error with
    
      /home/michael/Impala/bin/junitxml_command_wrapper.sh: line 46:
      DIST_DIR=/home/michael/Impala/shell/dist CLEAN_DIST=true
      /home/michael/Impala/shell/packaging/make_python_package.sh:
      No such file or directory
    
    Updates the default `DIST_DIR` to point to the documented path that
    CMake sets it to, and removes setting `DIST_DIR` as it matches the
    default value. Also removes `CLEAN_DIST` as that value is ignored.
    
    Change-Id: I60ffac3edf1a6027afa4ca46ab6dadfc6bfc660a
    Reviewed-on: http://gerrit.cloudera.org:8080/18578
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 CMakeLists.txt                         | 2 +-
 shell/packaging/make_python_package.sh | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3ca77fb38..857d295d6 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -463,7 +463,7 @@ add_custom_target(shell_tarball DEPENDS gen-deps
 )
 
 add_custom_target(shell_pypi_package DEPENDS shell_tarball
-  COMMAND "DIST_DIR=${CMAKE_SOURCE_DIR}/shell/dist CLEAN_DIST=true ${CMAKE_SOURCE_DIR}/shell/packaging/make_python_package.sh"
+  COMMAND "${CMAKE_SOURCE_DIR}/shell/packaging/make_python_package.sh"
 )
 
 add_custom_target(cscope ALL DEPENDS gen-deps
diff --git a/shell/packaging/make_python_package.sh b/shell/packaging/make_python_package.sh
index e04775d1b..9dee5bc60 100755
--- a/shell/packaging/make_python_package.sh
+++ b/shell/packaging/make_python_package.sh
@@ -22,7 +22,7 @@
 # a distributable python package of the Impala shell. The resulting
 # archive will be saved to:
 #
-#   ${IMPALA_HOME}/shell/dist/impala_shell-<version>.tar.gz
+#   ${DIST_DIR}/impala_shell-<version>.tar.gz
 #
 # Until the thrift-generated python files in ${IMPALA_HOME}/shell/gen-py
 # have been created by the build process, this script will not work.
@@ -37,7 +37,7 @@ set -eu -o pipefail
 WORKING_DIR="$(cd "$(dirname "$0")" ; pwd -P )"
 SHELL_HOME="${IMPALA_HOME}"/shell
 STAGING_DIR="${WORKING_DIR}"/staging
-DIST_DIR="${DIST_DIR:-$WORKING_DIR/dist}"
+DIST_DIR="${DIST_DIR:-$SHELL_HOME/dist}"
 PACKAGE_DIR="${STAGING_DIR}"/impala_shell_package
 MODULE_LIB_DIR="${PACKAGE_DIR}"/impala_shell
 NO_CLEAN_DIST="${NO_CLEAN_DIST:-}"
@@ -72,7 +72,7 @@ assemble_package_files() {
 }
 
 create_distributable_python_package() {
-  # Generate a new python package tarball in ${IMPALA_HOME}/shell/dist
+  # Generate a new python package tarball in ${DIST_DIR}
   if [[ "${NO_CLEAN_DIST}" != "true" ]]; then
     rm -rf "${DIST_DIR}"
   fi