IMPALA-8103: In Analyzed Query use /* and */ to delimit hints.

IMPALA-5821 added the analyzed query text to the output of EXPLAIN when
explain_level is 2 or greater. Any query hits were displayed in the form

SELECT
-- +straight_join
* FROM table_name [...]

which meant that care had to be taken to embed and preserve newlines in
the EXPLAIN output. This change makes the output to be of the form

SELECT /* +straight_join */ * FROM  table_name  [...]

The  /* +straight_join */ form was chosen over  /*+straight_join */ as
it seems more readable, and is more commonly used in exiting tests.

To do this the following changes were made:
ToSqlUtils.getPlanHintsSql() was extended to take a ToSqlOptions
parameter. If this parameter indicates that the analyzed query is being
shown, then the hints are surrounded with '/*' and '*/'.
PrintUtils.wrapString() was simplified as it no longer has to preserve
newlines in a an analyzed query.

TESTING
 All end-to-end tests were run.
 The output in a few .test files was updated.
 A couple of new cases were added to ToSqlTest.java

Change-Id: I7215a4c17508e3408680a1d2bb6c3af355c78c8d
Reviewed-on: http://gerrit.cloudera.org:8080/12360
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>

fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java

@@ -79,7 +79,7 @@ protected String tableRefToSql(ToSqlOptions options) {
     if (alias != null) aliasSql = " " + ToSqlUtils.getIdentSql(alias);
     String tableSampleSql = "";
     if (sampleParams_ != null) tableSampleSql = " " + sampleParams_.toSql(options);
-    String tableHintsSql = ToSqlUtils.getPlanHintsSql(tableHints_);
+    String tableHintsSql = ToSqlUtils.getPlanHintsSql(options, tableHints_);
     return getTable().getTableName().toSql() + aliasSql + tableSampleSql + tableHintsSql;
   }
 

fe/src/main/java/org/apache/impala/analysis/InsertStmt.java

@@ -928,7 +928,7 @@ public String toSql(ToSqlOptions options) {
 
     strBuilder.append(getOpName());
     if (!planHints_.isEmpty() && hintLoc_ == HintLocation.Start) {
-      strBuilder.append(" " + ToSqlUtils.getPlanHintsSql(getPlanHints()));
+      strBuilder.append(" " + ToSqlUtils.getPlanHintsSql(options, getPlanHints()));
     }
     if (overwrite_) {
       strBuilder.append(" OVERWRITE");
@@ -955,7 +955,7 @@ public String toSql(ToSqlOptions options) {
       strBuilder.append(" PARTITION (" + Joiner.on(", ").join(values) + ")");
     }
     if (!planHints_.isEmpty() && hintLoc_ == HintLocation.End) {
-      strBuilder.append(" " + ToSqlUtils.getPlanHintsSql(getPlanHints()));
+      strBuilder.append(" " + ToSqlUtils.getPlanHintsSql(options, getPlanHints()));
     }
     if (!needsGeneratedQueryStatement_) {
       strBuilder.append(" " + queryStmt_.toSql(options));

fe/src/main/java/org/apache/impala/analysis/SelectStmt.java

@@ -1051,7 +1051,8 @@ public String toSql(ToSqlOptions options) {
       strBuilder.append("DISTINCT ");
     }
     if (selectList_.hasPlanHints()) {
-      strBuilder.append(ToSqlUtils.getPlanHintsSql(selectList_.getPlanHints()) + " ");
+      strBuilder.append(ToSqlUtils.getPlanHintsSql(options, selectList_.getPlanHints()))
+          .append(" ");
     }
     for (int i = 0; i < selectList_.getItems().size(); ++i) {
       strBuilder.append(selectList_.getItems().get(i).toSql(options));

fe/src/main/java/org/apache/impala/analysis/TableRef.java

@@ -600,7 +600,8 @@ public String toSql(ToSqlOptions options) {
     }
 
     StringBuilder output = new StringBuilder(" " + joinOp_.toString() + " ");
-    if(!joinHints_.isEmpty()) output.append(ToSqlUtils.getPlanHintsSql(joinHints_) + " ");
+    if (!joinHints_.isEmpty())
+      output.append(ToSqlUtils.getPlanHintsSql(options, joinHints_)).append(" ");
     output.append(tableRefToSql(options));
     if (usingColNames_ != null) {
       output.append(" USING (").append(Joiner.on(", ").join(usingColNames_)).append(")");

fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java

@@ -533,13 +533,19 @@ public int compare(Entry<String, String> o1, Entry<String, String> o2) {
    * commented plan hint style such that hinted views created by Impala are readable by
    * Hive (parsed as a comment by Hive).
    */
-  public static String getPlanHintsSql(List<PlanHint> hints) {
+  public static String getPlanHintsSql(ToSqlOptions options, List<PlanHint> hints) {
     Preconditions.checkNotNull(hints);
     if (hints.isEmpty()) return "";
     StringBuilder sb = new StringBuilder();
-    sb.append("\n-- +");
-    sb.append(Joiner.on(",").join(hints));
-    sb.append("\n");
+    if (options.showRewritten()) {
+      sb.append("/* +");
+      sb.append(Joiner.on(",").join(hints));
+      sb.append(" */");
+    } else {
+      sb.append("\n-- +");
+      sb.append(Joiner.on(",").join(hints));
+      sb.append("\n");
+    }
     return sb.toString();
   }
 }

fe/src/main/java/org/apache/impala/common/PrintUtils.java

@@ -175,16 +175,9 @@ public static void printMatrix(
    * Any newlines in the input are maintained.
    */
   public static String wrapString(String s, int wrapLength) {
-    StringBuilder ret = new StringBuilder(s.length());
-    String[] split = s.split("\n");
-    for (int i = 0; i < split.length; i++) {
-      String line = split[i];
-      String wrappedLine = WordUtils.wrap(line, wrapLength, null, true);
-      // we keep any existing newlines in text - these should be commented hints
-      wrappedLine = wrappedLine.replaceAll(" +$", "");
-      ret.append(wrappedLine);
-      if (i < split.length - 1) ret.append("\n");
-    }
-    return ret.toString();
+    String wrapped = WordUtils.wrap(s, wrapLength, null, true);
+    // Remove any trailing blanks from a line.
+    wrapped = wrapped.replaceAll(" +$", "");
+    return wrapped;
   }
 }

fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java

@@ -82,6 +82,11 @@ private void testToSql(String query, String expected) {
     testToSql(query, System.getProperty("user.name"), expected);
   }
 
+  private void testToSql(String query, String expected, ToSqlOptions options) {
+    String defaultDb = System.getProperty("user.name");
+    testToSql(createAnalysisCtx(defaultDb), query, defaultDb, expected, false, options);
+  }
+
   private void testToSql(AnalysisContext ctx, String query, String expected) {
     testToSql(ctx, query, System.getProperty("user.name"), expected);
   }
@@ -92,23 +97,24 @@ private void testToSql(String query, String defaultDb, String expected) {
 
   private void testToSql(AnalysisContext ctx, String query, String defaultDb,
       String expected) {
-    testToSql(ctx, query, defaultDb, expected, false);
+    testToSql(ctx, query, defaultDb, expected, false, ToSqlOptions.DEFAULT);
   }
 
   private void testToSql(String query, String defaultDb, String expected,
       boolean ignoreWhitespace) {
-    testToSql(createAnalysisCtx(defaultDb), query, defaultDb, expected, ignoreWhitespace);
+    testToSql(createAnalysisCtx(defaultDb), query, defaultDb, expected, ignoreWhitespace,
+        ToSqlOptions.DEFAULT);
   }
 
   private void testToSql(AnalysisContext ctx, String query, String defaultDb,
-      String expected, boolean ignoreWhitespace) {
+      String expected, boolean ignoreWhitespace, ToSqlOptions options) {
     String actual = null;
     try {
       ParseNode node = AnalyzesOk(query, ctx);
-      if (node instanceof QueryStmt) {
+      if (node instanceof QueryStmt && !options.showRewritten()) {
         actual = ((QueryStmt)node).getOrigSqlString();
       } else {
-        actual = node.toSql();
+        actual = node.toSql(options);
       }
       if (ignoreWhitespace) {
         // Transform whitespace to single space.
@@ -686,6 +692,23 @@ public void planHintsTest() {
           String.format("select distinct %sstraight_join%s * from functional.alltypes",
           prefix, suffix),
           "SELECT DISTINCT \n-- +straight_join\n * FROM functional.alltypes");
+
+      // Tests for analyzed/rewritten sql.
+      // First test the test by passing ToSqlOptions.DEFAULT which should result in the
+      // hints appearing with '--' style comments.
+      testToSql(
+          String.format("select distinct %sstraight_join%s * from functional.alltypes",
+              prefix, suffix),
+          "SELECT DISTINCT \n-- +straight_join\n * FROM functional.alltypes",
+          ToSqlOptions.DEFAULT);
+      // Test that analyzed queries use the '/*' style comments.
+      testToSql(
+          String.format("select distinct %sstraight_join%s * from functional.alltypes "
+                  + "where bool_col = false and id <= 5 and id >= 2",
+              prefix, suffix),
+          "SELECT DISTINCT /* +straight_join */ * FROM functional.alltypes "
+              + "WHERE bool_col = FALSE AND id <= 5 AND id >= 2",
+          ToSqlOptions.REWRITTEN);
     }
   }
 

fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java

@@ -97,17 +97,6 @@ public void testWrapText() {
             + " AS DOUBLE) < CAST(10 AS DOUBLE)",
         "Analyzed query: SELECT * FROM functional_kudu.alltypestiny\n"
             + "WHERE CAST(bigint_col AS DOUBLE) < CAST(10 AS DOUBLE)");
-    // Simple query with a hint retains newlines surrounding hint.
-    assertWrap("SELECT\n"
-            + "-- +straight_join\n"
-            + " * FROM tpch_parquet.orders INNER JOIN\n"
-            + "-- +shuffle\n"
-            + " tpch_parquet.customer ON o_custkey = c_custkey",
-        "SELECT\n"
-            + "-- +straight_join\n"
-            + "* FROM tpch_parquet.orders INNER JOIN\n"
-            + "-- +shuffle\n"
-            + "tpch_parquet.customer ON o_custkey = c_custkey");
     // test that a long string of blanks prints OK, some may be lost for clarity
     assertWrap("insert into foo values ('                                      "
             + "                                                                          "

testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test

@@ -6,10 +6,8 @@ from tpch_parquet.customer
 ---- DISTRIBUTEDPLAN
 Max Per-Host Resource Reservation: Memory=33.97MB Threads=5
 Per-Host Resource Estimates: Memory=68MB
-Analyzed query: SELECT
--- +straight_join
-* FROM tpch_parquet.customer INNER JOIN tpch_parquet.nation ON c_nationkey =
-n_nationkey
+Analyzed query: SELECT /* +straight_join */ * FROM tpch_parquet.customer INNER
+JOIN tpch_parquet.nation ON c_nationkey = n_nationkey
 
 F02:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.33MB mem-reservation=0B thread-reservation=1
@@ -67,10 +65,8 @@ from tpch_parquet.lineitem
 ---- DISTRIBUTEDPLAN
 Max Per-Host Resource Reservation: Memory=110.00MB Threads=5
 Per-Host Resource Estimates: Memory=410MB
-Analyzed query: SELECT
--- +straight_join
-* FROM tpch_parquet.lineitem LEFT OUTER JOIN tpch_parquet.orders ON l_orderkey =
-o_orderkey
+Analyzed query: SELECT /* +straight_join */ * FROM tpch_parquet.lineitem LEFT
+OUTER JOIN tpch_parquet.orders ON l_orderkey = o_orderkey
 
 F02:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=11.20MB mem-reservation=0B thread-reservation=1
@@ -184,11 +180,10 @@ having count(*) = 1
 ---- DISTRIBUTEDPLAN
 Max Per-Host Resource Reservation: Memory=110.00MB Threads=7
 Per-Host Resource Estimates: Memory=272MB
-Analyzed query: SELECT
--- +straight_join
-l_orderkey, o_orderstatus, count(*) FROM tpch_parquet.lineitem INNER JOIN
-tpch_parquet.orders ON o_orderkey = l_orderkey GROUP BY l_orderkey,
-o_orderstatus HAVING count(*) = CAST(1 AS BIGINT)
+Analyzed query: SELECT /* +straight_join */ l_orderkey, o_orderstatus, count(*)
+FROM tpch_parquet.lineitem INNER JOIN tpch_parquet.orders ON o_orderkey =
+l_orderkey GROUP BY l_orderkey, o_orderstatus HAVING count(*) = CAST(1 AS
+BIGINT)
 
 F04:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.10MB mem-reservation=0B thread-reservation=1

testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test

@@ -2422,8 +2422,7 @@ from tpch.lineitem inner join /* +shuffle */ tpch.orders on l_orderkey = o_order
 ---- PLAN
 Max Per-Host Resource Reservation: Memory=51.00MB Threads=3
 Per-Host Resource Estimates: Memory=446MB
-Analyzed query: SELECT * FROM tpch.lineitem INNER JOIN
--- +shuffle
+Analyzed query: SELECT * FROM tpch.lineitem INNER JOIN /* +shuffle */
 tpch.orders ON l_orderkey = o_orderkey
 
 F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
@@ -2462,8 +2461,7 @@ PLAN-ROOT SINK
 ---- DISTRIBUTEDPLAN
 Max Per-Host Resource Reservation: Memory=52.00MB Threads=6
 Per-Host Resource Estimates: Memory=300MB
-Analyzed query: SELECT * FROM tpch.lineitem INNER JOIN
--- +shuffle
+Analyzed query: SELECT * FROM tpch.lineitem INNER JOIN /* +shuffle */
 tpch.orders ON l_orderkey = o_orderkey
 
 F03:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
@@ -2523,8 +2521,7 @@ Per-Host Resources: mem-estimate=89.00MB mem-reservation=9.00MB thread-reservati
 ---- PARALLELPLANS
 Max Per-Host Resource Reservation: Memory=104.00MB Threads=7
 Per-Host Resource Estimates: Memory=481MB
-Analyzed query: SELECT * FROM tpch.lineitem INNER JOIN
--- +shuffle
+Analyzed query: SELECT * FROM tpch.lineitem INNER JOIN /* +shuffle */
 tpch.orders ON l_orderkey = o_orderkey
 
 F03:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
@@ -4956,15 +4953,12 @@ join (
 ---- PLAN
 Max Per-Host Resource Reservation: Memory=99.00MB Threads=5
 Per-Host Resource Estimates: Memory=180MB
-Analyzed query: SELECT
--- +straight_join
-* FROM tpch_parquet.orders t1 INNER JOIN (SELECT
--- +straight_join
-t2.o_orderkey k2, k3, k4 FROM tpch_parquet.orders t2 INNER JOIN (SELECT
--- +straight_join
-t3.o_orderkey k3, t4.o_orderkey k4 FROM tpch_parquet.orders t3 INNER JOIN
-tpch_parquet.orders t4 ON t3.o_orderkey = t4.o_orderkey) v2 ON v2.k3 =
-t2.o_orderkey) v1 ON v1.k3 = t1.o_orderkey
+Analyzed query: SELECT /* +straight_join */ * FROM tpch_parquet.orders t1 INNER
+JOIN (SELECT /* +straight_join */ t2.o_orderkey k2, k3, k4 FROM
+tpch_parquet.orders t2 INNER JOIN (SELECT /* +straight_join */ t3.o_orderkey k3,
+t4.o_orderkey k4 FROM tpch_parquet.orders t3 INNER JOIN tpch_parquet.orders t4
+ON t3.o_orderkey = t4.o_orderkey) v2 ON v2.k3 = t2.o_orderkey) v1 ON v1.k3 =
+t1.o_orderkey
 
 F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=180.00MB mem-reservation=99.00MB thread-reservation=5 runtime-filters-memory=3.00MB
@@ -5040,15 +5034,12 @@ PLAN-ROOT SINK
 ---- DISTRIBUTEDPLAN
 Max Per-Host Resource Reservation: Memory=100.50MB Threads=10
 Per-Host Resource Estimates: Memory=260MB
-Analyzed query: SELECT
--- +straight_join
-* FROM tpch_parquet.orders t1 INNER JOIN (SELECT
--- +straight_join
-t2.o_orderkey k2, k3, k4 FROM tpch_parquet.orders t2 INNER JOIN (SELECT
--- +straight_join
-t3.o_orderkey k3, t4.o_orderkey k4 FROM tpch_parquet.orders t3 INNER JOIN
-tpch_parquet.orders t4 ON t3.o_orderkey = t4.o_orderkey) v2 ON v2.k3 =
-t2.o_orderkey) v1 ON v1.k3 = t1.o_orderkey
+Analyzed query: SELECT /* +straight_join */ * FROM tpch_parquet.orders t1 INNER
+JOIN (SELECT /* +straight_join */ t2.o_orderkey k2, k3, k4 FROM
+tpch_parquet.orders t2 INNER JOIN (SELECT /* +straight_join */ t3.o_orderkey k3,
+t4.o_orderkey k4 FROM tpch_parquet.orders t3 INNER JOIN tpch_parquet.orders t4
+ON t3.o_orderkey = t4.o_orderkey) v2 ON v2.k3 = t2.o_orderkey) v1 ON v1.k3 =
+t1.o_orderkey
 
 F05:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.41MB mem-reservation=0B thread-reservation=1
@@ -5159,15 +5150,12 @@ Per-Host Resources: mem-estimate=88.84MB mem-reservation=59.00MB thread-reservat
 ---- PARALLELPLANS
 Max Per-Host Resource Reservation: Memory=176.50MB Threads=11
 Per-Host Resource Estimates: Memory=454MB
-Analyzed query: SELECT
--- +straight_join
-* FROM tpch_parquet.orders t1 INNER JOIN (SELECT
--- +straight_join
-t2.o_orderkey k2, k3, k4 FROM tpch_parquet.orders t2 INNER JOIN (SELECT
--- +straight_join
-t3.o_orderkey k3, t4.o_orderkey k4 FROM tpch_parquet.orders t3 INNER JOIN
-tpch_parquet.orders t4 ON t3.o_orderkey = t4.o_orderkey) v2 ON v2.k3 =
-t2.o_orderkey) v1 ON v1.k3 = t1.o_orderkey
+Analyzed query: SELECT /* +straight_join */ * FROM tpch_parquet.orders t1 INNER
+JOIN (SELECT /* +straight_join */ t2.o_orderkey k2, k3, k4 FROM
+tpch_parquet.orders t2 INNER JOIN (SELECT /* +straight_join */ t3.o_orderkey k3,
+t4.o_orderkey k4 FROM tpch_parquet.orders t3 INNER JOIN tpch_parquet.orders t4
+ON t3.o_orderkey = t4.o_orderkey) v2 ON v2.k3 = t2.o_orderkey) v1 ON v1.k3 =
+t1.o_orderkey
 
 F05:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.82MB mem-reservation=0B thread-reservation=1
@@ -5318,14 +5306,11 @@ join (
 ---- PLAN
 Max Per-Host Resource Reservation: Memory=176.00KB Threads=5
 Per-Host Resource Estimates: Memory=138MB
-Analyzed query: SELECT
--- +straight_join
-* FROM tpch_parquet.nation t1 INNER JOIN (SELECT
--- +straight_join
-t2.n_nationkey k2, k3, k4 FROM tpch_parquet.nation t2 INNER JOIN (SELECT
--- +straight_join
-t3.n_nationkey k3, t4.s_suppkey k4 FROM tpch_parquet.nation t3 INNER JOIN
-tpch_parquet.supplier t4) v2) v1
+Analyzed query: SELECT /* +straight_join */ * FROM tpch_parquet.nation t1 INNER
+JOIN (SELECT /* +straight_join */ t2.n_nationkey k2, k3, k4 FROM
+tpch_parquet.nation t2 INNER JOIN (SELECT /* +straight_join */ t3.n_nationkey
+k3, t4.s_suppkey k4 FROM tpch_parquet.nation t3 INNER JOIN tpch_parquet.supplier
+t4) v2) v1
 
 F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=137.99MB mem-reservation=176.00KB thread-reservation=5
@@ -5389,14 +5374,11 @@ PLAN-ROOT SINK
 ---- DISTRIBUTEDPLAN
 Max Per-Host Resource Reservation: Memory=176.00KB Threads=9
 Per-Host Resource Estimates: Memory=161MB
-Analyzed query: SELECT
--- +straight_join
-* FROM tpch_parquet.nation t1 INNER JOIN (SELECT
--- +straight_join
-t2.n_nationkey k2, k3, k4 FROM tpch_parquet.nation t2 INNER JOIN (SELECT
--- +straight_join
-t3.n_nationkey k3, t4.s_suppkey k4 FROM tpch_parquet.nation t3 INNER JOIN
-tpch_parquet.supplier t4) v2) v1
+Analyzed query: SELECT /* +straight_join */ * FROM tpch_parquet.nation t1 INNER
+JOIN (SELECT /* +straight_join */ t2.n_nationkey k2, k3, k4 FROM
+tpch_parquet.nation t2 INNER JOIN (SELECT /* +straight_join */ t3.n_nationkey
+k3, t4.s_suppkey k4 FROM tpch_parquet.nation t3 INNER JOIN tpch_parquet.supplier
+t4) v2) v1
 
 F04:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.13MB mem-reservation=0B thread-reservation=1
@@ -5488,14 +5470,11 @@ Per-Host Resources: mem-estimate=97.55MB mem-reservation=32.00KB thread-reservat
 ---- PARALLELPLANS
 Max Per-Host Resource Reservation: Memory=352.00KB Threads=9
 Per-Host Resource Estimates: Memory=311MB
-Analyzed query: SELECT
--- +straight_join
-* FROM tpch_parquet.nation t1 INNER JOIN (SELECT
--- +straight_join
-t2.n_nationkey k2, k3, k4 FROM tpch_parquet.nation t2 INNER JOIN (SELECT
--- +straight_join
-t3.n_nationkey k3, t4.s_suppkey k4 FROM tpch_parquet.nation t3 INNER JOIN
-tpch_parquet.supplier t4) v2) v1
+Analyzed query: SELECT /* +straight_join */ * FROM tpch_parquet.nation t1 INNER
+JOIN (SELECT /* +straight_join */ t2.n_nationkey k2, k3, k4 FROM
+tpch_parquet.nation t2 INNER JOIN (SELECT /* +straight_join */ t3.n_nationkey
+k3, t4.s_suppkey k4 FROM tpch_parquet.nation t3 INNER JOIN tpch_parquet.supplier
+t4) v2) v1
 
 F04:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.27MB mem-reservation=0B thread-reservation=1