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 2018/07/17 16:53:22 UTC

[2/7] impala git commit: IMPALA-7106: Log the original and rewritten SQL when SQL rewrite fails

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>


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

Branch: refs/heads/master
Commit: b1ec9e375f6a2104585b1acae29f9fed407679f6
Parents: 2ddff52
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Fri Jun 1 00:11:11 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Tue Jul 17 04:36:59 2018 +0000

----------------------------------------------------------------------
 .../apache/impala/analysis/AnalysisContext.java |  13 ++-
 .../analysis/CreateTableAsSelectStmt.java       |   4 +-
 .../org/apache/impala/analysis/DeleteStmt.java  |   8 +-
 .../org/apache/impala/analysis/FromClause.java  |   6 +-
 .../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 +--
 .../apache/impala/analysis/StatementBase.java   |   8 +-
 .../apache/impala/analysis/StmtRewriter.java    |   3 +-
 .../org/apache/impala/analysis/TableRef.java    |  12 +-
 .../org/apache/impala/analysis/ToSqlUtils.java  |  10 +-
 .../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 +-
 .../impala/analysis/ExprRewriterTest.java       | 114 +++++++++++++++++++
 17 files changed, 203 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
----------------------------------------------------------------------
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

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
----------------------------------------------------------------------
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) {

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
----------------------------------------------------------------------
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());

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/FromClause.java
----------------------------------------------------------------------
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();

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
----------------------------------------------------------------------
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.

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
----------------------------------------------------------------------
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 3483d28..32efc2c 100644
--- a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
@@ -895,10 +895,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) {
@@ -927,7 +927,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();
   }

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
----------------------------------------------------------------------
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);
 }

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
----------------------------------------------------------------------
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 fda44cf..6156f0b 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
@@ -934,17 +934,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(" ");
     }
 
@@ -961,7 +958,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 ");

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
----------------------------------------------------------------------
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_; }
 

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
----------------------------------------------------------------------
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));
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/TableRef.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/TableRef.java b/fe/src/main/java/org/apache/impala/analysis/TableRef.java
index 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) {

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
----------------------------------------------------------------------
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);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
----------------------------------------------------------------------
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()) {

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
----------------------------------------------------------------------
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 ");

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/main/java/org/apache/impala/analysis/WithClause.java
----------------------------------------------------------------------
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);
   }

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
----------------------------------------------------------------------
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 " +

http://git-wip-us.apache.org/repos/asf/impala/blob/b1ec9e37/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
----------------------------------------------------------------------
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));
+  }
 }