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/29 15:33:56 UTC

[impala] branch 2.x updated (97ca8d1 -> 2413c6c)

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

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


    from 97ca8d1  IMPALA-7299: [DOCS] A known issue with IMPALA-7298
     new 046d9f8  [DOCS] A typo fix in impala_perf_stats.xml
     new 1ee17ee  IMPALA-7106: Log the original and rewritten SQL when SQL rewrite fails
     new b41a780  IMPALA-7132: Filter out useless output from run_clang_tidy.sh
     new 7a86ac9  Remove dead multiple_filesystems member from THdfsTable
     new 68caba1  IMPALA-7238: Use custom timeout for create unique database
     new 1e946fb  IMPALA-7199: Add scripts to create code coverage reports
     new 2413c6c  IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option

The 7 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:
 bin/bootstrap_toolchain.py                         |   2 +-
 bin/coverage_helper.sh                             |  86 ++++++++++++++++
 bin/impala-config.sh                               |   8 +-
 bin/{impala-flake8 => impala-gcovr}                |   2 +-
 bin/run_clang_tidy.sh                              |  10 +-
 common/thrift/CatalogObjects.thrift                |   4 +-
 docs/impala.ditamap                                |   1 +
 docs/impala_keydefs.ditamap                        |   4 +-
 docs/topics/impala_kudu_read_mode.xml              |  88 ++++++++++++++++
 docs/topics/impala_perf_stats.xml                  |   2 +-
 .../apache/impala/analysis/AnalysisContext.java    |  13 ++-
 .../impala/analysis/CreateTableAsSelectStmt.java   |   4 +-
 .../org/apache/impala/analysis/DeleteStmt.java     |   8 +-
 .../org/apache/impala/analysis/FromClause.java     |   6 +-
 .../org/apache/impala/analysis/InlineViewRef.java  |   7 +-
 .../org/apache/impala/analysis/InsertStmt.java     |   6 +-
 .../org/apache/impala/analysis/ModifyStmt.java     |   9 +-
 .../org/apache/impala/analysis/SelectStmt.java     |  13 ++-
 .../org/apache/impala/analysis/StatementBase.java  |   8 +-
 .../org/apache/impala/analysis/StmtRewriter.java   |   3 +-
 .../java/org/apache/impala/analysis/TableRef.java  |  12 ++-
 .../org/apache/impala/analysis/ToSqlUtils.java     |  10 +-
 .../java/org/apache/impala/analysis/UnionStmt.java |  15 +--
 .../org/apache/impala/analysis/UpdateStmt.java     |  10 +-
 .../org/apache/impala/analysis/WithClause.java     |   6 +-
 .../java/org/apache/impala/catalog/HdfsTable.java  |   7 --
 .../apache/impala/catalog/local/LocalFsTable.java  |   1 -
 .../impala/analysis/AnalyzeSubqueriesTest.java     |   2 +-
 .../apache/impala/analysis/ExprRewriterTest.java   | 114 +++++++++++++++++++++
 infra/python/deps/requirements.txt                 |   1 +
 testdata/bin/kill-all.sh                           |   3 +
 tests/conftest.py                                  |   4 +-
 32 files changed, 404 insertions(+), 65 deletions(-)
 create mode 100755 bin/coverage_helper.sh
 copy bin/{impala-flake8 => impala-gcovr} (96%)
 create mode 100644 docs/topics/impala_kudu_read_mode.xml


[impala] 06/07: IMPALA-7199: Add scripts to create code coverage reports

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

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

commit 1e946fbbea0d15e4022088e57f162cc81015ac15
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Thu May 24 14:58:16 2018 -0700

    IMPALA-7199: Add scripts to create code coverage reports
    
    gcovr is a python library that uses gcov to generate
    code coverage reports. This adds gcovr to the python
    dependencies and adds bin/impala-gcovr to provide
    easy access to gcovr's command line. gcovr 3.4
    supports python 2.6+.
    
    This also adds bin/coverage_helper.sh to provide a
    simplified interface to generate reports and zero
    coverage counters.
    
    Code coverage data is written out when a program
    exits, so it is important to avoid hard kills
    to shut down the impalads when generating coverage.
    This modifies testdata/bin/kill-all.sh to call
    start-impala-cluster.py --kill when shutting down
    the minicluster to try to avoid doing a hard kill.
    It will still do a hard kill if impala is still
    running after the softer kill.
    
    Change-Id: I5b2e0b794c64f9343ec976de7a3f235e54d2badd
    Reviewed-on: http://gerrit.cloudera.org:8080/10791
    Reviewed-by: Joe McDonnell <jo...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 bin/coverage_helper.sh                       | 86 ++++++++++++++++++++++++++++
 testdata/bin/kill-all.sh => bin/impala-gcovr | 18 +-----
 infra/python/deps/requirements.txt           |  1 +
 testdata/bin/kill-all.sh                     |  3 +
 4 files changed, 93 insertions(+), 15 deletions(-)

diff --git a/bin/coverage_helper.sh b/bin/coverage_helper.sh
new file mode 100755
index 0000000..dad1498
--- /dev/null
+++ b/bin/coverage_helper.sh
@@ -0,0 +1,86 @@
+#!/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.
+
+set -eu -o pipefail
+
+ZERO_COUNTERS_ACTION=0
+REPORT_ACTION=0
+REPORT_DIRECTORY=${IMPALA_HOME}/logs/coverage
+
+function print_usage {
+  echo "coverage_helper.sh - Driver script for coverage"
+  echo "[-zerocounters] : Reset coverage counters"
+  echo "[-report] : Generate a detailed coverage report"
+  echo "[-reportdirectory <directory>] : Output directory for coverage report files"
+}
+
+while [ -n "$*" ]
+do
+  case "$1" in
+    -zerocounters)
+       ZERO_COUNTERS_ACTION=1
+       ;;
+    -report)
+       REPORT_ACTION=1
+       ;;
+    -reportdirectory)
+       REPORT_DIRECTORY="${2-}"
+       shift
+       ;;
+    -help|*)
+       print_usage
+       exit 1
+       ;;
+  esac
+  shift
+done
+
+if [[ ${ZERO_COUNTERS_ACTION} -eq 0 && ${REPORT_ACTION} -eq 0 ]]; then
+  print_usage
+  exit 1
+fi
+
+if pgrep -U "$USER" impalad; then
+  echo "Warning: impalad is running. Coverage counters are only updated when"
+  echo "a program exits. Any report will not include the information from"
+  echo "the running impalad. Similarly, zeroing the counters will not zero"
+  echo "the counters for the running impalad."
+fi
+
+if [ ${REPORT_ACTION} -eq 1 ]; then
+  mkdir -p "${REPORT_DIRECTORY}"
+  rm -f "${REPORT_DIRECTORY}"/index*.html
+  # src/util/bit-packing.inline.h gets lots of hits, so generating a detailed report
+  # for it takes several minutes. Exclude it to keep the execution time down.
+  # gcovr excludes are buggy, so on some environments these excludes won't work.
+  echo "Generating report in directory: ${REPORT_DIRECTORY}"
+  cd "${IMPALA_HOME}"
+  "${IMPALA_HOME}/bin/impala-gcovr" -v -r "${IMPALA_HOME}/be" \
+    --exclude=".*src/benchmarks.*" \
+    --exclude=".*generated-sources/gen-cpp.*" \
+    --exclude=".*src/util/bit-packing.inline.h.*" \
+    --html --html-details -o "${REPORT_DIRECTORY}/index.html" > "${REPORT_DIRECTORY}/gcovr.out"
+fi
+
+if [ ${ZERO_COUNTERS_ACTION} -eq 1 ]; then
+  # The .gcda files contain the counters for coverage. Deleting them zeros the
+  # the counters.
+  echo "Zeroing Counters"
+  find "${IMPALA_HOME}/be" -name "*.gcda" -delete
+fi
diff --git a/testdata/bin/kill-all.sh b/bin/impala-gcovr
similarity index 60%
copy from testdata/bin/kill-all.sh
copy to bin/impala-gcovr
index 5462162..6e8be6c 100755
--- a/testdata/bin/kill-all.sh
+++ b/bin/impala-gcovr
@@ -1,5 +1,5 @@
 #!/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
@@ -17,17 +17,5 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -euo pipefail
-trap 'echo Error in $0 at line $LINENO: $(cd "'$PWD'" && awk "NR == $LINENO" $0)' ERR
-
-# Kill HBase, then MiniLlama (which includes a MiniDfs, a Yarn RM several NMs).
-$IMPALA_HOME/testdata/bin/kill-sentry-service.sh
-$IMPALA_HOME/testdata/bin/kill-hive-server.sh
-$IMPALA_HOME/testdata/bin/kill-hbase.sh
-$IMPALA_HOME/testdata/bin/kill-mini-dfs.sh
-
-for BINARY in impalad statestored catalogd mini-impalad-cluster; do
-  if pgrep -U $USER $BINARY; then
-    killall -9 -u $USER -q $BINARY
-  fi
-done
+source "$(dirname "$0")/impala-python-common.sh"
+exec "$PY_DIR/env/bin/gcovr" "$@"
diff --git a/infra/python/deps/requirements.txt b/infra/python/deps/requirements.txt
index b0b7f23..a92fdc6 100644
--- a/infra/python/deps/requirements.txt
+++ b/infra/python/deps/requirements.txt
@@ -43,6 +43,7 @@ Flask == 0.10.1
   MarkupSafe == 0.23
   Werkzeug == 0.11.3
   itsdangerous == 0.24
+gcovr == 3.4
 kazoo == 2.2.1
 ordereddict == 1.1
 pexpect == 3.3
diff --git a/testdata/bin/kill-all.sh b/testdata/bin/kill-all.sh
index 5462162..0e8d201 100755
--- a/testdata/bin/kill-all.sh
+++ b/testdata/bin/kill-all.sh
@@ -20,6 +20,9 @@
 set -euo pipefail
 trap 'echo Error in $0 at line $LINENO: $(cd "'$PWD'" && awk "NR == $LINENO" $0)' ERR
 
+# Shutdown Impala if it is alive
+${IMPALA_HOME}/bin/start-impala-cluster.py --kill
+
 # Kill HBase, then MiniLlama (which includes a MiniDfs, a Yarn RM several NMs).
 $IMPALA_HOME/testdata/bin/kill-sentry-service.sh
 $IMPALA_HOME/testdata/bin/kill-hive-server.sh


[impala] 03/07: IMPALA-7132: Filter out useless output from run_clang_tidy.sh

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

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

commit b41a780065b60e1e807099937f06dd328652cc87
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Wed Jun 6 12:25:56 2018 -0700

    IMPALA-7132: Filter out useless output from run_clang_tidy.sh
    
    Clang's run-clang-tidy.py script produces a lot of
    output even when there are no warnings or errors.
    None of this output is useful.
    
    This patch has two parts:
    1. Bump LLVM to 5.0.1-p1, which has patched run-clang-tidy.py
       to make it reduce its own output when passed -quiet
       (along with other enhancements).
    2. Pass -quiet to run-clang-tidy.py and pipe the stderr output
       to a temporary file. Display this output only if
       run-clang-tidy.py hits an error, as this output is not
       useful otherwise.
    
    Testing with a known clang tidy issue shows that warnings
    and errors are still in the output, and the output is
    clean on a clean Impala checkout.
    
    Change-Id: I63c46a7d57295eba38fac8ab49c7a15d2802df1d
    Reviewed-on: http://gerrit.cloudera.org:8080/10615
    Reviewed-by: Jim Apple <jb...@apache.org>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 bin/bootstrap_toolchain.py |  2 +-
 bin/impala-config.sh       |  8 ++++----
 bin/run_clang_tidy.sh      | 10 +++++++++-
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/bin/bootstrap_toolchain.py b/bin/bootstrap_toolchain.py
index 0c2a63a..96547fd 100755
--- a/bin/bootstrap_toolchain.py
+++ b/bin/bootstrap_toolchain.py
@@ -425,7 +425,7 @@ if __name__ == "__main__":
       "flatbuffers", "gcc", "gflags", "glog", "gperftools", "gtest", "libev", "libunwind",
       "lz4", "openldap", "openssl", "orc", "protobuf",
       "rapidjson", "re2", "snappy", "thrift", "tpc-h", "tpc-ds", "zlib"])
-  packages.insert(0, Package("llvm", "5.0.1-asserts"))
+  packages.insert(0, Package("llvm", "5.0.1-asserts-p1"))
   bootstrap(toolchain_root, packages)
 
   # Download the CDH components if necessary.
diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index d62d37a..d6ea425 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -68,7 +68,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=137-93cacec18d
+export IMPALA_TOOLCHAIN_BUILD_ID=146-f2d5380be6
 # Versions of toolchain dependencies.
 # -----------------------------------
 export IMPALA_AVRO_VERSION=1.7.4-p4
@@ -103,14 +103,14 @@ export IMPALA_LIBEV_VERSION=4.20
 unset IMPALA_LIBEV_URL
 export IMPALA_LIBUNWIND_VERSION=1.3-rc1-p3
 unset IMPALA_LIBUNWIND_URL
-export IMPALA_LLVM_VERSION=5.0.1
+export IMPALA_LLVM_VERSION=5.0.1-p1
 unset IMPALA_LLVM_URL
-export IMPALA_LLVM_ASAN_VERSION=5.0.1
+export IMPALA_LLVM_ASAN_VERSION=5.0.1-p1
 unset IMPALA_LLVM_ASAN_URL
 
 # Debug builds should use the release+asserts build to get additional coverage.
 # Don't use the LLVM debug build because the binaries are too large to distribute.
-export IMPALA_LLVM_DEBUG_VERSION=5.0.1-asserts
+export IMPALA_LLVM_DEBUG_VERSION=5.0.1-asserts-p1
 unset IMPALA_LLVM_DEBUG_URL
 export IMPALA_LZ4_VERSION=1.7.5
 unset IMPALA_LZ4_URL
diff --git a/bin/run_clang_tidy.sh b/bin/run_clang_tidy.sh
index e879b35..a0e022a 100755
--- a/bin/run_clang_tidy.sh
+++ b/bin/run_clang_tidy.sh
@@ -54,4 +54,12 @@ fi
 export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/share/clang\
 :${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin/\
 :$PATH"
-run-clang-tidy.py -header-filter "${PIPE_DIRS%?}" -j"${CORES}" ${DIRS}
+TMP_STDERR=$(mktemp)
+trap "rm $TMP_STDERR" EXIT
+if ! run-clang-tidy.py -quiet -header-filter "${PIPE_DIRS%?}" \
+                       -j"${CORES}" ${DIRS} 2> ${TMP_STDERR};
+then
+  echo "run-clang-tidy.py hit an error, dumping stderr output"
+  cat ${TMP_STDERR} >&2
+  exit 1
+fi


[impala] 01/07: [DOCS] A typo fix in impala_perf_stats.xml

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

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

commit 046d9f8984446849c78aab37139d28cdf67cefee
Author: Alex Rodoni <ar...@cloudera.com>
AuthorDate: Mon Jul 16 19:22:44 2018 -0700

    [DOCS] A typo fix in impala_perf_stats.xml
    
    Change-Id: I2de22e61ce016cb9c1f780bca84a7338482b212d
    Reviewed-on: http://gerrit.cloudera.org:8080/10958
    Reviewed-by: Alex Rodoni <ar...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 docs/topics/impala_perf_stats.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/topics/impala_perf_stats.xml b/docs/topics/impala_perf_stats.xml
index 5e5d487..b7106a4 100644
--- a/docs/topics/impala_perf_stats.xml
+++ b/docs/topics/impala_perf_stats.xml
@@ -476,7 +476,7 @@ show column stats year_month_day;
             Set the impalad start-up configuration "--enable_stats_extrapolation" to
             enable the features globally. To enable them only for a specific table, set
             the "impala.enable.stats.extrapolation" table property to "true" for the
-            desired table. The tbale-level property overrides the global setting, so
+            desired table. The table-level property overrides the global setting, so
             it is also possible to enable sampling and extrapolation globally, but
             disable it for specific tables by setting the table property to "false".
             Example:


[impala] 02/07: IMPALA-7106: Log the original and rewritten SQL when SQL rewrite fails

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

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

commit 1ee17ee89a579e65c2fbc55a613ac368d01161a0
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Fri Jun 1 00:11:11 2018 -0700

    IMPALA-7106: Log the original and rewritten SQL when SQL rewrite fails
    
    toSql() method is used to print SQL string that is close to the original
    SQL string when errors arise or as the result of "SHOW CREATE". When
    debugging issues related to SQL rewrites, it can be very useful to be
    able to get the SQL string that is being rewritten. This patch adds a
    new method toSql(boolean rewritten) to get the rewritten SQL string. This
    patch also logs the original and rewritten SQL when SQL rewrite fails.
    
    Testing:
    - Added FE test for the rewritten SQL string
    - Ran all FE tests
    
    Change-Id: Iab58b0cc865135d261dd4a7f72be130f2e7bde53
    Reviewed-on: http://gerrit.cloudera.org:8080/10571
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../apache/impala/analysis/AnalysisContext.java    |  13 ++-
 .../impala/analysis/CreateTableAsSelectStmt.java   |   4 +-
 .../org/apache/impala/analysis/DeleteStmt.java     |   8 +-
 .../org/apache/impala/analysis/FromClause.java     |   6 +-
 .../org/apache/impala/analysis/InlineViewRef.java  |   7 +-
 .../org/apache/impala/analysis/InsertStmt.java     |   6 +-
 .../org/apache/impala/analysis/ModifyStmt.java     |   9 +-
 .../org/apache/impala/analysis/SelectStmt.java     |  13 ++-
 .../org/apache/impala/analysis/StatementBase.java  |   8 +-
 .../org/apache/impala/analysis/StmtRewriter.java   |   3 +-
 .../java/org/apache/impala/analysis/TableRef.java  |  12 ++-
 .../org/apache/impala/analysis/ToSqlUtils.java     |  10 +-
 .../java/org/apache/impala/analysis/UnionStmt.java |  15 +--
 .../org/apache/impala/analysis/UpdateStmt.java     |  10 +-
 .../org/apache/impala/analysis/WithClause.java     |   6 +-
 .../impala/analysis/AnalyzeSubqueriesTest.java     |   2 +-
 .../apache/impala/analysis/ExprRewriterTest.java   | 114 +++++++++++++++++++++
 17 files changed, 203 insertions(+), 43 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
index 9ad0807..eeda2eb 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -456,7 +456,7 @@ public class AnalysisContext {
       // reset() and re-analyzed. For a CTAS statement, the types represent column types
       // of the table that will be created, including the partition columns, if any.
       List<Type> origResultTypes = Lists.newArrayList();
-      for (Expr e: analysisResult_.stmt_.getResultExprs()) {
+      for (Expr e : analysisResult_.stmt_.getResultExprs()) {
         origResultTypes.add(e.getType());
       }
       List<String> origColLabels =
@@ -472,13 +472,20 @@ public class AnalysisContext {
       // Re-analyze the stmt with a new analyzer.
       analysisResult_.analyzer_ = createAnalyzer(stmtTableCache);
       analysisResult_.stmt_.reset();
-      analysisResult_.stmt_.analyze(analysisResult_.analyzer_);
+      try {
+        analysisResult_.stmt_.analyze(analysisResult_.analyzer_);
+      } catch (AnalysisException e) {
+        LOG.error(String.format("Error analyzing the rewritten query.\n" +
+            "Original SQL: %s\nRewritten SQL: %s", analysisResult_.stmt_.toSql(),
+            analysisResult_.stmt_.toSql(true)));
+        throw e;
+      }
 
       // Restore the original result types and column labels.
       analysisResult_.stmt_.castResultExprs(origResultTypes);
       analysisResult_.stmt_.setColLabels(origColLabels);
       if (LOG.isTraceEnabled()) {
-        LOG.trace("rewrittenStmt: " + analysisResult_.stmt_.toSql());
+        LOG.trace("Rewritten SQL: " + analysisResult_.stmt_.toSql(true));
       }
 
       // Restore privilege requests found during the first pass
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
index 3e03ef4..76b63c9 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
@@ -108,7 +108,9 @@ public class CreateTableAsSelectStmt extends StatementBase {
   public InsertStmt getInsertStmt() { return insertStmt_; }
   public CreateTableStmt getCreateStmt() { return createStmt_; }
   @Override
-  public String toSql() { return ToSqlUtils.getCreateTableSql(this); }
+  public String toSql(boolean rewritten) {
+    return ToSqlUtils.getCreateTableSql(this, rewritten);
+  }
 
   @Override
   public void collectTableRefs(List<TableRef> tblRefs) {
diff --git a/fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java b/fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
index 52d58a7..3542548 100644
--- a/fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
@@ -68,7 +68,9 @@ public class DeleteStmt extends ModifyStmt {
   }
 
   @Override
-  public String toSql() {
+  public String toSql(boolean rewritten) {
+    if (!rewritten && sqlString_ != null) return sqlString_;
+
     StringBuilder b = new StringBuilder();
     b.append("DELETE");
     if (fromClause_.size() > 1 || targetTableRef_.hasExplicitAlias()) {
@@ -76,10 +78,10 @@ public class DeleteStmt extends ModifyStmt {
       if (targetTableRef_.hasExplicitAlias()) {
         b.append(targetTableRef_.getExplicitAlias());
       } else {
-        b.append(targetTableRef_.toSql());
+        b.append(targetTableRef_.toSql(rewritten));
       }
     }
-    b.append(fromClause_.toSql());
+    b.append(fromClause_.toSql(rewritten));
     if (wherePredicate_ != null) {
       b.append(" WHERE ");
       b.append(wherePredicate_.toSql());
diff --git a/fe/src/main/java/org/apache/impala/analysis/FromClause.java b/fe/src/main/java/org/apache/impala/analysis/FromClause.java
index 294c324..10a021e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/FromClause.java
+++ b/fe/src/main/java/org/apache/impala/analysis/FromClause.java
@@ -113,11 +113,15 @@ public class FromClause implements ParseNode, Iterable<TableRef> {
 
   @Override
   public String toSql() {
+    return toSql(false);
+  }
+
+  public String toSql(boolean rewritten) {
     StringBuilder builder = new StringBuilder();
     if (!tableRefs_.isEmpty()) {
       builder.append(" FROM ");
       for (int i = 0; i < tableRefs_.size(); ++i) {
-        builder.append(tableRefs_.get(i).toSql());
+        builder.append(tableRefs_.get(i).toSql(rewritten));
       }
     }
     return builder.toString();
diff --git a/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java b/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
index 4fee2fa..dbcb59a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
+++ b/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
@@ -324,6 +324,11 @@ public class InlineViewRef extends TableRef {
 
   @Override
   protected String tableRefToSql() {
+    return tableRefToSql(false);
+  }
+
+  @Override
+  protected String tableRefToSql(boolean rewritten) {
     // Enclose the alias in quotes if Hive cannot parse it without quotes.
     // This is needed for view compatibility between Impala and Hive.
     String aliasSql = null;
@@ -335,7 +340,7 @@ public class InlineViewRef extends TableRef {
     Preconditions.checkNotNull(aliasSql);
     StringBuilder sql = new StringBuilder()
         .append("(")
-        .append(queryStmt_.toSql())
+        .append(queryStmt_.toSql(rewritten))
         .append(") ")
         .append(aliasSql);
     // Add explicit col labels for debugging even though this syntax isn't supported.
diff --git a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
index 493f14b..487da52 100644
--- a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
@@ -889,10 +889,10 @@ public class InsertStmt extends StatementBase {
   }
 
   @Override
-  public String toSql() {
+  public String toSql(boolean rewritten) {
     StringBuilder strBuilder = new StringBuilder();
 
-    if (withClause_ != null) strBuilder.append(withClause_.toSql() + " ");
+    if (withClause_ != null) strBuilder.append(withClause_.toSql(rewritten) + " ");
 
     strBuilder.append(getOpName());
     if (!planHints_.isEmpty() && hintLoc_ == HintLocation.Start) {
@@ -921,7 +921,7 @@ public class InsertStmt extends StatementBase {
       strBuilder.append(" " + ToSqlUtils.getPlanHintsSql(getPlanHints()));
     }
     if (!needsGeneratedQueryStatement_) {
-      strBuilder.append(" " + queryStmt_.toSql());
+      strBuilder.append(" " + queryStmt_.toSql(rewritten));
     }
     return strBuilder.toString();
   }
diff --git a/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java b/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
index ec24685..29266f6 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
@@ -89,6 +89,9 @@ public abstract class ModifyStmt extends StatementBase {
   // position in the target table. Set in createSourceStmt() during analysis.
   protected ArrayList<Integer> referencedColumns_;
 
+  // SQL string of the ModifyStmt. Set in analyze().
+  protected String sqlString_;
+
   public ModifyStmt(List<String> targetTablePath, FromClause fromClause,
       List<Pair<SlotRef, Expr>> assignmentExprs, Expr wherePredicate) {
     targetTablePath_ = Preconditions.checkNotNull(targetTablePath);
@@ -166,6 +169,8 @@ public abstract class ModifyStmt extends StatementBase {
     sourceStmt_.analyze(analyzer);
     // Add target table to descriptor table.
     analyzer.getDescTbl().setTargetTable(table_);
+
+    sqlString_ = toSql();
   }
 
   @Override
@@ -311,7 +316,5 @@ public abstract class ModifyStmt extends StatementBase {
   public QueryStmt getQueryStmt() { return sourceStmt_; }
   public abstract DataSink createDataSink();
   @Override
-  public abstract String toSql();
-
-
+  public abstract String toSql(boolean rewritten);
 }
diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
index d7ec04f..3c0260d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
@@ -912,17 +912,14 @@ public class SelectStmt extends QueryStmt {
     }
   }
 
-  /**
-   * Returns the SQL string corresponding to this SelectStmt.
-   */
   @Override
-  public String toSql() {
+  public String toSql(boolean rewritten) {
     // Return the SQL string before inline-view expression substitution.
-    if (sqlString_ != null) return sqlString_;
+    if (!rewritten && sqlString_ != null) return sqlString_;
 
     StringBuilder strBuilder = new StringBuilder();
     if (withClause_ != null) {
-      strBuilder.append(withClause_.toSql());
+      strBuilder.append(withClause_.toSql(rewritten));
       strBuilder.append(" ");
     }
 
@@ -939,7 +936,9 @@ public class SelectStmt extends QueryStmt {
       strBuilder.append((i+1 != selectList_.getItems().size()) ? ", " : "");
     }
     // From clause
-    if (!fromClause_.isEmpty()) { strBuilder.append(fromClause_.toSql()); }
+    if (!fromClause_.isEmpty()) {
+      strBuilder.append(fromClause_.toSql(rewritten));
+    }
     // Where clause
     if (whereClause_ != null) {
       strBuilder.append(" WHERE ");
diff --git a/fe/src/main/java/org/apache/impala/analysis/StatementBase.java b/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
index c770742..c4609de 100644
--- a/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
+++ b/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
@@ -130,7 +130,13 @@ public abstract class StatementBase implements ParseNode {
   public Analyzer getAnalyzer() { return analyzer_; }
   public boolean isAnalyzed() { return analyzer_ != null; }
 
-  public String toSql() { return ""; }
+  public String toSql() { return toSql(false); }
+  /**
+   * If rewritten is true, returns the rewritten SQL only if the statement was
+   * rewritten. Otherwise, the original SQL will be returned instead. It is the caller's
+   * responsibility to know if/when the statement was indeed rewritten.
+   */
+  public String toSql(boolean rewritten) { return ""; }
   public void setIsExplain() { isExplain_ = true; }
   public boolean isExplain() { return isExplain_; }
 
diff --git a/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java b/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
index 4206e99..edf380d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
+++ b/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
@@ -97,8 +97,7 @@ public class StmtRewriter {
     // Currently only SubqueryRewriter touches the where clause. Recurse into the where
     // clause when the need arises.
     rewriteSelectStmtHook(stmt, analyzer);
-    stmt.sqlString_ = null;
-    if (LOG.isTraceEnabled()) LOG.trace("rewritten stmt: " + stmt.toSql());
+    if (LOG.isTraceEnabled()) LOG.trace("Rewritten SQL: " + stmt.toSql(true));
   }
 
   /**
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 86cc25b..98861d1 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TableRef.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TableRef.java
@@ -572,17 +572,25 @@ public class TableRef implements ParseNode {
     return ToSqlUtils.getPathSql(path) + ((aliasSql != null) ? " " + aliasSql : "");
   }
 
+  protected String tableRefToSql(boolean rewritten) {
+    return tableRefToSql();
+  }
+
   @Override
   public String toSql() {
+    return toSql(false);
+  }
+
+  public String toSql(boolean rewritten) {
     if (joinOp_ == null) {
       // prepend "," if we're part of a sequence of table refs w/o an
       // explicit JOIN clause
-      return (leftTblRef_ != null ? ", " : "") + tableRefToSql();
+      return (leftTblRef_ != null ? ", " : "") + tableRefToSql(rewritten);
     }
 
     StringBuilder output = new StringBuilder(" " + joinOp_.toString() + " ");
     if(!joinHints_.isEmpty()) output.append(ToSqlUtils.getPlanHintsSql(joinHints_) + " ");
-    output.append(tableRefToSql());
+    output.append(tableRefToSql(rewritten));
     if (usingColNames_ != null) {
       output.append(" USING (").append(Joiner.on(", ").join(usingColNames_)).append(")");
     } else if (onClause_ != null) {
diff --git a/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java b/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
index 444e211..cdd67ce 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
@@ -186,9 +186,13 @@ public class ToSqlUtils {
 
   /**
    * Returns the "CREATE TABLE" SQL string corresponding to the given
-   * CreateTableAsSelectStmt statement.
+   * CreateTableAsSelectStmt statement. If rewritten is true, returns the rewritten SQL
+   * only if the statement was rewritten. Otherwise, the original SQL will be returned
+   * instead. It is the caller's responsibility to know if/when the statement was indeed
+   * rewritten.
    */
-  public static String getCreateTableSql(CreateTableAsSelectStmt stmt) {
+  public static String getCreateTableSql(CreateTableAsSelectStmt stmt,
+      boolean rewritten) {
     CreateTableStmt innerStmt = stmt.getCreateStmt();
     // Only add partition column labels to output. Table columns must not be specified as
     // they are deduced from the select statement.
@@ -212,7 +216,7 @@ public class ToSqlUtils {
         innerStmt.isExternal(), innerStmt.getIfNotExists(), innerStmt.getRowFormat(),
         HdfsFileFormat.fromThrift(innerStmt.getFileFormat()), HdfsCompression.NONE, null,
         innerStmt.getLocation());
-    return createTableSql + " AS " + stmt.getQueryStmt().toSql();
+    return createTableSql + " AS " + stmt.getQueryStmt().toSql(rewritten);
   }
 
   /**
diff --git a/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java b/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
index 1e58fad..313479e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
@@ -551,24 +551,25 @@ public class UnionStmt extends QueryStmt {
   }
 
   @Override
-  public String toSql() {
-    if (toSqlString_ != null) return toSqlString_;
+  public String toSql(boolean rewritten) {
+    if (!rewritten && toSqlString_ != null) return toSqlString_;
+
     StringBuilder strBuilder = new StringBuilder();
     Preconditions.checkState(operands_.size() > 0);
 
     if (withClause_ != null) {
-      strBuilder.append(withClause_.toSql());
+      strBuilder.append(withClause_.toSql(rewritten));
       strBuilder.append(" ");
     }
 
-    strBuilder.append(operands_.get(0).getQueryStmt().toSql());
+    strBuilder.append(operands_.get(0).getQueryStmt().toSql(rewritten));
     for (int i = 1; i < operands_.size() - 1; ++i) {
       strBuilder.append(" UNION " +
           ((operands_.get(i).getQualifier() == Qualifier.ALL) ? "ALL " : ""));
       if (operands_.get(i).getQueryStmt() instanceof UnionStmt) {
         strBuilder.append("(");
       }
-      strBuilder.append(operands_.get(i).getQueryStmt().toSql());
+      strBuilder.append(operands_.get(i).getQueryStmt().toSql(rewritten));
       if (operands_.get(i).getQueryStmt() instanceof UnionStmt) {
         strBuilder.append(")");
       }
@@ -583,10 +584,10 @@ public class UnionStmt extends QueryStmt {
             !lastQueryStmt.hasLimit() && !lastQueryStmt.hasOffset() &&
             !lastQueryStmt.hasOrderByClause())) {
       strBuilder.append("(");
-      strBuilder.append(lastQueryStmt.toSql());
+      strBuilder.append(lastQueryStmt.toSql(rewritten));
       strBuilder.append(")");
     } else {
-      strBuilder.append(lastQueryStmt.toSql());
+      strBuilder.append(lastQueryStmt.toSql(rewritten));
     }
     // Order By clause
     if (hasOrderByClause()) {
diff --git a/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java b/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
index ddce618..3a977b7 100644
--- a/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
@@ -77,17 +77,19 @@ public class UpdateStmt extends ModifyStmt {
   }
 
   @Override
-  public String toSql() {
+  public String toSql(boolean rewritten) {
+    if (!rewritten && sqlString_ != null) return sqlString_;
+
     StringBuilder b = new StringBuilder();
     b.append("UPDATE ");
 
     if (fromClause_ == null) {
-      b.append(targetTableRef_.toSql());
+      b.append(targetTableRef_.toSql(rewritten));
     } else {
       if (targetTableRef_.hasExplicitAlias()) {
         b.append(targetTableRef_.getExplicitAlias());
       } else {
-        b.append(targetTableRef_.toSql());
+        b.append(targetTableRef_.toSql(rewritten));
       }
     }
     b.append(" SET");
@@ -104,7 +106,7 @@ public class UpdateStmt extends ModifyStmt {
           i.second.toSql()));
     }
 
-    b.append(fromClause_.toSql());
+    b.append(fromClause_.toSql(rewritten));
 
     if (wherePredicate_ != null) {
       b.append(" WHERE ");
diff --git a/fe/src/main/java/org/apache/impala/analysis/WithClause.java b/fe/src/main/java/org/apache/impala/analysis/WithClause.java
index 4f93f36..d253d91 100644
--- a/fe/src/main/java/org/apache/impala/analysis/WithClause.java
+++ b/fe/src/main/java/org/apache/impala/analysis/WithClause.java
@@ -117,6 +117,10 @@ public class WithClause implements ParseNode {
 
   @Override
   public String toSql() {
+    return toSql(false);
+  }
+
+  public String toSql(boolean rewritten) {
     List<String> viewStrings = Lists.newArrayList();
     for (View view: views_) {
       // Enclose the view alias and explicit labels in quotes if Hive cannot parse it
@@ -126,7 +130,7 @@ public class WithClause implements ParseNode {
         aliasSql += "(" + Joiner.on(", ").join(
             ToSqlUtils.getIdentSqlList(view.getOriginalColLabels())) + ")";
       }
-      viewStrings.add(aliasSql + " AS (" + view.getQueryStmt().toSql() + ")");
+      viewStrings.add(aliasSql + " AS (" + view.getQueryStmt().toSql(rewritten) + ")");
     }
     return "WITH " + Joiner.on(",").join(viewStrings);
   }
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
index 06e834d..6e19189 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
@@ -711,7 +711,7 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
         " where a.int_col between t.tinyint_col and t.bigint_col)",
         "Unsupported predicate with subquery: " +
         "EXISTS (SELECT id FROM functional.alltypessmall a " +
-        "WHERE a.int_col >= t.tinyint_col AND a.int_col <= t.bigint_col)");
+        "WHERE a.int_col BETWEEN t.tinyint_col AND t.bigint_col)");
 
     // Uncorrelated EXISTS in a query with GROUP BY
     AnalyzesOk("select id, count(*) from functional.alltypes t " +
diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
index de58d31..20d4fd9 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
@@ -24,6 +24,7 @@ import org.apache.impala.common.RuntimeEnv;
 import org.apache.impala.rewrite.EqualityDisjunctsToInRule;
 import org.apache.impala.rewrite.ExprRewriteRule;
 import org.apache.impala.rewrite.ExprRewriter;
+import org.apache.impala.thrift.TQueryOptions;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -223,4 +224,117 @@ public class ExprRewriterTest extends AnalyzerTest {
       CheckNumChangesByEqualityDisjunctsToInRule(stmtSb.toString(), 1);
     }
   }
+
+  @Test
+  public void TestToSql() {
+    TQueryOptions options = new TQueryOptions();
+    options.setEnable_expr_rewrites(true);
+    AnalysisContext ctx = createAnalysisCtx(options);
+
+    //----------------------
+    // Test query rewrites.
+    //----------------------
+    assertToSql(ctx, "select 1 + 1", "SELECT 1 + 1", "SELECT 2");
+
+    assertToSql(ctx,
+        "select (case when true then 1 else id end) from functional.alltypes " +
+        "union " +
+        "select 1 + 1",
+        "SELECT (CASE WHEN TRUE THEN 1 ELSE id END) FROM functional.alltypes " +
+        "UNION " +
+        "SELECT 1 + 1",
+        "SELECT 1 FROM functional.alltypes UNION SELECT 2");
+
+    assertToSql(ctx,
+        "values(1, '2', 3, 4.1), (1, '2', 3, 4.1)",
+        "VALUES((1, '2', 3, 4.1), (1, '2', 3, 4.1))",
+        "SELECT 1, '2', 3, 4.1 UNION ALL SELECT 1, '2', 3, 4.1");
+
+    //-------------------------
+    // Test subquery rewrites.
+    //-------------------------
+    assertToSql(ctx, "select * from (" +
+        "select * from functional.alltypes where id = (select 1 + 1)) a",
+        "SELECT * FROM (SELECT * FROM functional.alltypes WHERE id = (SELECT 1 + 1)) a",
+        "SELECT * FROM (SELECT * FROM functional.alltypes LEFT SEMI JOIN " +
+        "(SELECT 2) `$a$1` (`$c$1`) ON id = `$a$1`.`$c$1`) a");
+
+    assertToSql(ctx,
+        "select * from (select * from functional.alltypes where id = (select 1 + 1)) a " +
+        "union " +
+        "select * from (select * from functional.alltypes where id = (select 1 + 1)) b",
+        "SELECT * FROM (SELECT * FROM functional.alltypes WHERE id = (SELECT 1 + 1)) a " +
+        "UNION " +
+        "SELECT * FROM (SELECT * FROM functional.alltypes WHERE id = (SELECT 1 + 1)) b",
+        "SELECT * FROM (SELECT * FROM functional.alltypes LEFT SEMI JOIN (SELECT 2) " +
+        "`$a$1` (`$c$1`) ON id = `$a$1`.`$c$1`) a " +
+        "UNION " +
+        "SELECT * FROM (SELECT * FROM functional.alltypes LEFT SEMI JOIN (SELECT 2) " +
+        "`$a$1` (`$c$1`) ON id = `$a$1`.`$c$1`) b");
+
+    assertToSql(ctx, "select * from " +
+        "(select (case when true then 1 else id end) from functional.alltypes " +
+        "union select 1 + 1) v",
+        "SELECT * FROM (SELECT (CASE WHEN TRUE THEN 1 ELSE id END) " +
+        "FROM functional.alltypes UNION SELECT 1 + 1) v",
+        "SELECT * FROM (SELECT 1 FROM functional.alltypes " +
+        "UNION SELECT 2) v");
+
+    //---------------------
+    // Test CTAS rewrites.
+    //---------------------
+    assertToSql(ctx,
+        "create table ctas_test as select 1 + 1",
+        "CREATE TABLE default.ctas_test\n" +
+        "STORED AS TEXTFILE\n" +
+        " AS SELECT 1 + 1",
+        "CREATE TABLE default.ctas_test\n" +
+        "STORED AS TEXTFILE\n" +
+        " AS SELECT 2");
+
+    //--------------------
+    // Test DML rewrites.
+    //--------------------
+    // Insert
+    assertToSql(ctx,
+        "insert into functional.alltypes(id) partition(year=2009, month=10) " +
+        "select 1 + 1",
+        "INSERT INTO TABLE functional.alltypes(id) " +
+        "PARTITION (year=2009, month=10) SELECT 1 + 1",
+        "INSERT INTO TABLE functional.alltypes(id) " +
+        "PARTITION (year=2009, month=10) SELECT 2");
+
+    if (RuntimeEnv.INSTANCE.isKuduSupported()) {
+      // Update.
+      assertToSql(ctx, "update functional_kudu.alltypes " +
+          "set string_col = 'test' where id = (select 1 + 1)",
+          "UPDATE functional_kudu.alltypes SET string_col = 'test' " +
+          "FROM functional_kudu.alltypes WHERE id = (SELECT 1 + 1)",
+          "UPDATE functional_kudu.alltypes SET string_col = 'test' " +
+          "FROM functional_kudu.alltypes LEFT SEMI JOIN (SELECT 2) `$a$1` (`$c$1`) " +
+          "ON id = `$a$1`.`$c$1` WHERE id = (SELECT 1 + 1)");
+
+      // Delete
+      assertToSql(ctx, "delete functional_kudu.alltypes " +
+          "where id = (select 1 + 1)",
+          "DELETE FROM functional_kudu.alltypes " +
+          "WHERE id = (SELECT 1 + 1)",
+          "DELETE functional_kudu.alltypes " +
+          "FROM functional_kudu.alltypes LEFT SEMI JOIN (SELECT 2) `$a$1` (`$c$1`) " +
+          "ON id = `$a$1`.`$c$1` WHERE id = (SELECT 1 + 1)");
+    }
+
+    // We don't do any rewrite for WITH clause.
+    StatementBase stmt = (StatementBase) AnalyzesOk("with t as (select 1 + 1) " +
+        "select id from functional.alltypes union select id from functional.alltypesagg",
+        ctx);
+    Assert.assertEquals(stmt.toSql(), stmt.toSql(true));
+  }
+
+  private void assertToSql(AnalysisContext ctx, String query, String expectedToSql,
+      String expectedToRewrittenSql) {
+    StatementBase stmt = (StatementBase) AnalyzesOk(query, ctx);
+    Assert.assertEquals(expectedToSql, stmt.toSql());
+    Assert.assertEquals(expectedToRewrittenSql, stmt.toSql(true));
+  }
 }


[impala] 05/07: IMPALA-7238: Use custom timeout for create unique database

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

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

commit 68caba1313a1380d9d99acf7aed209b62e43dbb6
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Tue Jul 3 16:20:15 2018 -0700

    IMPALA-7238: Use custom timeout for create unique database
    
    test_kudu.TestCreateExternalTables() saw a timeout when
    creating the unique database for its tests.
    
    __unique_conn() opens a connection, creates a unique database,
    then returns another connection in that database. It takes
    a custom timeout argument, but the timeout is only for the
    returned connection. The first connection to create the
    unique database uses the default timeout of 45 seconds.
    
    This patch changes the first connection to use the custom
    timeout. For Kudu tests, this is 5 minutes rather than 45
    seconds.
    
    Change-Id: I4f2beb5bc027a4bb44e854bf1dd8919807a92ea0
    Reviewed-on: http://gerrit.cloudera.org:8080/10862
    Reviewed-by: Joe McDonnell <jo...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 tests/conftest.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/conftest.py b/tests/conftest.py
index fe6b332..a1f1859 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -356,7 +356,7 @@ def conn(request):
     with __unique_conn(db_name=db_name, timeout=timeout) as conn:
       yield conn
   else:
-    with __auto_closed_conn(db_name=db_name) as conn:
+    with __auto_closed_conn(db_name=db_name, timeout=timeout) as conn:
       yield conn
 
 
@@ -388,7 +388,7 @@ def __unique_conn(db_name=None, timeout=DEFAULT_CONN_TIMEOUT):
   """
   if not db_name:
     db_name = choice(ascii_lowercase) + "".join(sample(ascii_lowercase + digits, 5))
-  with __auto_closed_conn() as conn:
+  with __auto_closed_conn(timeout=timeout) as conn:
     with __auto_closed_cursor(conn) as cur:
       cur.execute("CREATE DATABASE %s" % db_name)
   with __auto_closed_conn(db_name=db_name, timeout=timeout) as conn:


[impala] 04/07: Remove dead multiple_filesystems member from THdfsTable

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

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

commit 7a86ac9c51a4e3380c6ef7cbbdd5bd580eec6561
Author: Todd Lipcon <to...@cloudera.com>
AuthorDate: Thu Jul 12 16:17:07 2018 -0700

    Remove dead multiple_filesystems member from THdfsTable
    
    This member was added long ago with a TODO to remove it. The member has
    not been in use for quite some time, so this patch removes the member as
    well as its assignment.
    
    Change-Id: I33197a52e05d0c9647f5ce64a77c59950d9a1b94
    Reviewed-on: http://gerrit.cloudera.org:8080/10935
    Reviewed-by: Vuk Ercegovac <ve...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 common/thrift/CatalogObjects.thrift                                | 4 +---
 fe/src/main/java/org/apache/impala/catalog/HdfsTable.java          | 7 -------
 fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java | 1 -
 3 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/common/thrift/CatalogObjects.thrift b/common/thrift/CatalogObjects.thrift
index bc068d6..d463300 100644
--- a/common/thrift/CatalogObjects.thrift
+++ b/common/thrift/CatalogObjects.thrift
@@ -314,9 +314,7 @@ struct THdfsTable {
   // than duplicate the list of network address, which helps reduce memory usage.
   7: optional list<Types.TNetworkAddress> network_addresses
 
-  // Indicates that this table's partitions reside on more than one filesystem.
-  // TODO: remove once INSERT across filesystems is supported.
-  8: optional bool multiple_filesystems
+  // REMOVED: 8: optional bool multiple_filesystems
 
   // The prefixes of locations of partitions in this table. See THdfsPartitionLocation for
   // the description of how a prefix is computed.
diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
index 8ab33d8..5360960 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
@@ -205,9 +205,6 @@ public class HdfsTable extends Table implements FeFsTable {
   // True iff this table has incremental stats in any of its partitions.
   private boolean hasIncrementalStats_ = false;
 
-  // True iff the table's partitions are located on more than one filesystem.
-  private boolean multipleFileSystems_ = false;
-
   private HdfsPartitionLocationCompressor partitionLocationCompressor_;
 
   // Base Hdfs directory where files of this table are stored.
@@ -1077,8 +1074,6 @@ public class HdfsTable extends Table implements FeFsTable {
     Path partDirPath = new Path(storageDescriptor.getLocation());
     try {
       FileSystem fs = partDirPath.getFileSystem(CONF);
-      multipleFileSystems_ = multipleFileSystems_ ||
-          !FileSystemUtil.isPathOnFileSystem(new Path(getLocation()), fs);
       if (msPartition != null) {
         HdfsCachingUtil.validateCacheParams(msPartition.getParameters());
       }
@@ -1667,7 +1662,6 @@ public class HdfsTable extends Table implements FeFsTable {
     hdfsBaseDir_ = hdfsTable.getHdfsBaseDir();
     nullColumnValue_ = hdfsTable.nullColumnValue;
     nullPartitionKeyValue_ = hdfsTable.nullPartitionKeyValue;
-    multipleFileSystems_ = hdfsTable.multiple_filesystems;
     hostIndex_.populate(hdfsTable.getNetwork_addresses());
     resetPartitions();
     try {
@@ -1764,7 +1758,6 @@ public class HdfsTable extends Table implements FeFsTable {
     THdfsTable hdfsTable = new THdfsTable(hdfsBaseDir_, getColumnNames(),
         nullPartitionKeyValue_, nullColumnValue_, idToPartition, prototypePartition);
     hdfsTable.setAvroSchema(avroSchema_);
-    hdfsTable.setMultiple_filesystems(multipleFileSystems_);
     if (includeFileDesc) {
       // Network addresses are used only by THdfsFileBlocks which are inside
       // THdfsFileDesc, so include network addreses only when including THdfsFileDesc.
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
index 8a6c0f6..b652e0d 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
@@ -187,7 +187,6 @@ public class LocalFsTable extends LocalTable implements FeFsTable {
         /*includeIncrementalStats=*/false);
 
     // TODO(todd): implement avro schema support
-    // TODO(todd): set multiple_filesystems member?
     THdfsTable hdfsTable = new THdfsTable(getHdfsBaseDir(), getColumnNames(),
         getNullPartitionKeyValue(), schemaInfo_.getNullColumnValue(), idToPartition,
         tPrototypePartition);


[impala] 07/07: IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option

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

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

commit 2413c6c0b5a78ca4e8d918319f9986fa75eec66b
Author: Alex Rodoni <ar...@cloudera.com>
AuthorDate: Mon Jul 9 19:42:03 2018 -0700

    IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option
    
    Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952
    Reviewed-on: http://gerrit.cloudera.org:8080/10897
    Reviewed-by: Alex Rodoni <ar...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-on: http://gerrit.cloudera.org:8080/13455
---
 docs/impala.ditamap                   |  1 +
 docs/impala_keydefs.ditamap           |  4 +-
 docs/topics/impala_kudu_read_mode.xml | 88 +++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/docs/impala.ditamap b/docs/impala.ditamap
index 228da2e..8872fa1 100644
--- a/docs/impala.ditamap
+++ b/docs/impala.ditamap
@@ -195,6 +195,7 @@ under the License.
           <topicref href="topics/impala_explain_level.xml"/>
           <topicref href="topics/impala_hbase_cache_blocks.xml"/>
           <topicref href="topics/impala_hbase_caching.xml"/>
+          <topicref href="topics/impala_kudu_read_mode.xml"/>
           <topicref href="topics/impala_live_progress.xml"/>
           <topicref href="topics/impala_live_summary.xml"/>
           <topicref href="topics/impala_max_errors.xml"/>
diff --git a/docs/impala_keydefs.ditamap b/docs/impala_keydefs.ditamap
index 9cce199..b216f02 100644
--- a/docs/impala_keydefs.ditamap
+++ b/docs/impala_keydefs.ditamap
@@ -10521,6 +10521,7 @@ under the License.
   <keydef href="https://issues.apache.org/jira/browse/IMPALA-9999" scope="external" format="html" keys="IMPALA-9999"/>
 
 <!-- Short form of mapping from Impala release to vendor-specific releases, for use in headings. -->
+  <keydef keys="impala31"><topicmeta><keywords><keyword>Impala 3.1</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala30"><topicmeta><keywords><keyword>Impala 3.0</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala212"><topicmeta><keywords><keyword>Impala 2.12</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala211"><topicmeta><keywords><keyword>Impala 2.11</keyword></keywords></topicmeta></keydef>
@@ -10540,6 +10541,7 @@ under the License.
 
 <!-- 3-part forms of version numbers, for use in release notes. -->
 <!-- Using spaced-out form to avoid conflict with variable for 2.1.10 -->
+  <keydef keys="impala3_1_0"><topicmeta><keywords><keyword>Impala 3.1.0</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala3_00_0"><topicmeta><keywords><keyword>Impala 3.0.0</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala2_12_0"><topicmeta><keywords><keyword>Impala 2.12.0</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala2_11_0"><topicmeta><keywords><keyword>Impala 2.11.0</keyword></keywords></topicmeta></keydef>
@@ -10581,7 +10583,7 @@ under the License.
   <keydef keys="impala132"><topicmeta><keywords><keyword>Impala 1.3.2</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala130"><topicmeta><keywords><keyword>Impala 1.3.0</keyword></keywords></topicmeta></keydef>
 
-<!-- Long form of mapping from Impala release to vendor-specific releases, for use in running text. -->
+  <keydef keys="impala31_full"><topicmeta><keywords><keyword>Impala 3.1</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala30_full"><topicmeta><keywords><keyword>Impala 3.0</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala213_full"><topicmeta><keywords><keyword>Impala 2.13</keyword></keywords></topicmeta></keydef>
   <keydef keys="impala212_full"><topicmeta><keywords><keyword>Impala 2.12</keyword></keywords></topicmeta></keydef>
diff --git a/docs/topics/impala_kudu_read_mode.xml b/docs/topics/impala_kudu_read_mode.xml
new file mode 100644
index 0000000..a2d34b0
--- /dev/null
+++ b/docs/topics/impala_kudu_read_mode.xml
@@ -0,0 +1,88 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+<!DOCTYPE concept PUBLIC "-//OASIS//DTD DITA Concept//EN" "concept.dtd">
+<concept id="kudu_read_mode" rev="3.1.0 IMPALA-6812">
+
+  <title>KUDU_READ_MODE Query Option (<keyword keyref="impala31"/> or higher only)</title>
+
+  <titlealts audience="PDF">
+
+    <navtitle>KUDU_READ_MODE</navtitle>
+
+  </titlealts>
+
+  <prolog>
+    <metadata>
+      <data name="Category" value="Impala"/>
+      <data name="Category" value="Impala Query Options"/>
+      <data name="Category" value="Performance"/>
+      <data name="Category" value="Developers"/>
+      <data name="Category" value="Data Analysts"/>
+    </metadata>
+  </prolog>
+
+  <conbody>
+
+    <p rev="3.1.0 IMPALA-6812">
+      The <codeph>KUDU_READ_MODE</codeph> query option allows you to set a desired consistency
+      level for scans of Kudu tables.
+    </p>
+
+    <p>
+      <b>Type:</b> String
+    </p>
+
+    <p>
+      <b>Default:</b> <codeph>"DEFAULT"</codeph>
+    </p>
+
+    <p>
+      <b>Added in:</b> <keyword keyref="impala31"/>
+    </p>
+
+    <p conref="../shared/impala_common.xml#common/usage_notes_blurb"/>
+
+    <p>
+      The following values are supported for the query option:
+      <ul>
+        <li>
+          <codeph>"DEFAULT"</codeph>: The value of the startup flag,
+          <codeph>--kudu_read_mode</codeph>, is used.
+        </li>
+
+        <li>
+          <codeph>"READ_LATEST"</codeph>: Commonly known as the Read Committed isolation mode,
+          in this mode, Kudu provides no consistency guarantees for this mode, except that all
+          returned rows were committed at some point.
+        </li>
+
+        <li>
+          <codeph>"READ_AT_SNAPSHOT"</codeph>: Kudu will take a snapshot of the current state of
+          the data and perform the scan over the snapshot, possibly after briefly waiting for
+          ongoing writes to complete. This provides "Read Your Writes" consistency within a
+          single Impala session, except in the case of a Kudu leader change. See the Kudu
+          documentation for more details.
+        </li>
+      </ul>
+    </p>
+
+  </conbody>
+
+</concept>