HIVE-29217: Add configuration to choose materialization strategy for CTEs (#6092)

Author: zabetak

common/src/java/org/apache/hadoop/hive/conf/CteSuggesterType.java

@@ -0,0 +1,44 @@
+/*
+ * 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.
+ */
+package org.apache.hadoop.hive.conf;
+
+/**
+ * Type of suggester used for common table expression (CTE) detection and materialization.
+ */
+public enum CteSuggesterType {
+  /**
+   * Materialization is based on the AST/SQL structure of the query. The suggester only works when the
+   * query explicitly defines CTEs using WITH clauses. The suggester applies early during the syntactic analysis phase
+   * of the query and materializes WITH clauses into tables using heuristics and configured thresholds.
+   */
+  AST,
+  /**
+   * Materialization is based on the algebraic structure of the query. The suggester applies during the cost-based
+   * optimization phase and the exact behavior can be configured via
+   * {@link org.apache.hadoop.hive.conf.HiveConf.ConfVars#HIVE_CTE_SUGGESTER_CLASS} property.
+   */
+  CBO,
+  /**
+   * Materialization is disabled.
+   */
+  NONE;
+
+  public boolean enabled(HiveConf conf) {
+    return this.name().equalsIgnoreCase(conf.getVar(HiveConf.ConfVars.HIVE_CTE_SUGGESTER_TYPE));
+  }
+}

common/src/java/org/apache/hadoop/hive/conf/HiveConf.java

@@ -2654,8 +2654,15 @@ public static enum ConfVars {
 
     // CTE
     @InterfaceStability.Unstable
-    HIVE_CTE_SUGGESTER_CLASS("hive.optimize.cte.suggester.class", "",
-        "Class for finding and suggesting common table expressions (CTEs) based on a given query. The class must implement the CommonTableExpressionSuggester interface."),
+    HIVE_CTE_SUGGESTER_TYPE("hive.optimize.cte.suggester.type", "AST", new StringSet("AST", "CBO", "NONE"),
+        "The type of the suggester that is used for finding and materializing common table expressions " +
+            "(CTEs) based on a given query."),
+    @InterfaceStability.Unstable
+    HIVE_CTE_SUGGESTER_CLASS("hive.optimize.cte.suggester.class",
+        "org.apache.hadoop.hive.ql.optimizer.calcite.CommonTableExpressionIdentitySuggester",
+        "The class implementing the common table expression (CTE) suggester logic. This configuration is " +
+            "only relevant for the CBO suggester. The class must implement the CommonTableExpressionSuggester " +
+            "interface."),
     HIVE_CTE_MATERIALIZE_THRESHOLD("hive.optimize.cte.materialize.threshold", 3,
         "If the number of references to a CTE clause exceeds this threshold, Hive will materialize it\n" +
         "before executing the main query block. -1 will disable this feature."),

common/src/test/org/apache/hadoop/hive/conf/TestHiveConfVarsValidate.java

@@ -25,6 +25,7 @@
 import java.util.List;
 
 import static org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.HIVE_CTE_SUGGESTER_TYPE;
 import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.HIVE_DATETIME_FORMATTER;
 import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.HIVE_DATETIME_RESOLVER_STYLE;
 import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.HIVE_EXPLAIN_NODE_VISIT_LIMIT;
@@ -71,6 +72,13 @@ public static Collection<Object[]> generateParameters() {
     list.add(new Object[] { HIVE_DATETIME_RESOLVER_STYLE, "lenient", null});
     list.add(new Object[] { HIVE_DATETIME_RESOLVER_STYLE, "OTHER", "Invalid value.. expects one of [smart, strict, " +
             "lenient]" });
+    list.add(new Object[] {HIVE_CTE_SUGGESTER_TYPE, "AST", null});
+    list.add(new Object[] {HIVE_CTE_SUGGESTER_TYPE, "ast", null});
+    list.add(new Object[] {HIVE_CTE_SUGGESTER_TYPE, "CBO", null});
+    list.add(new Object[] {HIVE_CTE_SUGGESTER_TYPE, "cbo", null});
+    list.add(new Object[] {HIVE_CTE_SUGGESTER_TYPE, "NONE", null});
+    list.add(new Object[] {HIVE_CTE_SUGGESTER_TYPE, "nOnE", null});
+    list.add(new Object[] {HIVE_CTE_SUGGESTER_TYPE, "OTHER", "Invalid value.. expects one of [ast, cbo, none]"});
     return list;
   }
 

data/conf/perf/tpcds30tb/cte/hive-site.xml

@@ -26,7 +26,6 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameters;
-import org.junit.runners.model.Statement;
 
 import java.io.File;
 import java.util.List;
@@ -44,27 +43,8 @@ public static List<Object[]> getParameters() throws Exception {
   @ClassRule
   public static TestRule cliClassRule = adapter.buildClassRule();
 
-  /**
-   * Rule for calling only {@link CliAdapter#setUp()} and {@link CliAdapter#tearDown()} before/after running each test.
-   *
-   * At the moment of writing this class the rule is mostly necessary for calling {@link CliAdapter#tearDown()} to avoid
-   * state from one test pass to other (e.g., disabling one test should not disable subsequent ones).
-   *
-   * {@link CliAdapter#buildTestRule()} cannot not used since it is doing more than necessary for this test case. For
-   * instance, we do not want to create and destroy the metastore after each query.
-   */
   @Rule
-  public TestRule cliTestRule = (statement, description) -> new Statement() {
-    @Override
-    public void evaluate() throws Throwable {
-      adapter.setUp();
-      try {
-        statement.evaluate();
-      } finally {
-        adapter.tearDown();
-      }
-    }
-  };
+  public TestRule cliTestRule = adapter.buildTestRule();
 
   private final String name;
   private final File qfile;

data/conf/perf/tpcds30tb/cte/tez-site.xml

@@ -347,7 +347,15 @@ public TPCDSCteCliConfig() {
       setQueryDir("ql/src/test/queries/clientpositive/perf");
       setLogDir("itests/qtest/target/qfile-results/clientpositive/perf/tpcds30tb/cte");
       setResultsDir("ql/src/test/results/clientpositive/perf/tpcds30tb/cte");
-      setHiveConfDir("data/conf/perf/tpcds30tb/cte");
+      setHiveConfDir("data/conf/perf/tpcds30tb/tez");
+      Map<HiveConf.ConfVars, String> conf = new EnumMap<>(HiveConf.ConfVars.class);
+      conf.put(HiveConf.ConfVars.HIVE_CTE_SUGGESTER_TYPE, "CBO");
+      conf.put(
+          HiveConf.ConfVars.HIVE_CTE_SUGGESTER_CLASS,
+          "org.apache.hadoop.hive.ql.optimizer.calcite.CommonTableExpressionPrintSuggester");
+      conf.put(HiveConf.ConfVars.HIVE_CTE_MATERIALIZE_THRESHOLD, "1");
+      conf.put(HiveConf.ConfVars.HIVE_CTE_MATERIALIZE_FULL_AGGREGATE_ONLY, "false");
+      setCustomConfigValueMap(conf);
       setClusterType(MiniClusterType.LLAP_LOCAL);
       setMetastoreType("postgres.tpcds");
       // At the moment only makes sense to check CBO plans

itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestTPCDSCteCliDriver.java

@@ -22,6 +22,7 @@
 import org.apache.hadoop.hive.common.classification.InterfaceAudience;
 import org.apache.hadoop.hive.ql.session.SessionState;
 
+import java.util.Comparator;
 import java.util.List;
 
 /**
@@ -35,9 +36,9 @@ public class CommonTableExpressionPrintSuggester implements CommonTableExpressio
   @Override
   public List<RelNode> suggest(final RelNode input, final Configuration configuration) {
     List<RelNode> result = internal.suggest(input, configuration);
-    // Ensure CTEs are printed deterministically to avoid test flakiness
-    result.stream().map(RelOptUtil::toString).sorted()
-        .forEach(cte -> SessionState.getConsole().printInfo("CTE Suggestion:\n" + cte, false));
+    // Ensure CTEs are printed and returned deterministically to avoid test flakiness
+    result.sort(Comparator.comparing(RelOptUtil::toString));
+    result.forEach(cte -> SessionState.getConsole().printInfo("CTE Suggestion:\n" + RelOptUtil.toString(cte), false));
     return result;
   }
 

itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java

@@ -135,6 +135,7 @@
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hive.common.TableName;
 import org.apache.hadoop.hive.conf.Constants;
+import org.apache.hadoop.hive.conf.CteSuggesterType;
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
 import org.apache.hadoop.hive.conf.HiveConf.StrictChecks;
@@ -636,7 +637,7 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept
             // unfortunately making prunedPartitions immutable is not possible
             // here with SemiJoins not all tables are costed in CBO, so their
             // PartitionList is not evaluated until the run phase.
-            getMetaData(getQB(), true);
+            getMetaData(getQB(), CteSuggesterType.CBO.enabled(conf));
 
             disableJoinMerge = defaultJoinMerge;
             sinkOp = genPlan(getQB());
@@ -1727,7 +1728,7 @@ public RelNode apply(RelOptCluster cluster, RelOptSchema relOptSchema, SchemaPlu
       perfLogger.perfLogEnd(this.getClass().getName(), PerfLogger.POSTJOIN_ORDERING);
       // Perform the CTE rewriting near the end of CBO transformations to avoid interference of the new HiveTableSpool 
       // operator with other rules (especially those related to constant folding and branch pruning).
-      if (!forViewCreation) {
+      if (!forViewCreation && CteSuggesterType.CBO.enabled(conf)) {
         calcitePlan = applyCteRewriting(planner, calcitePlan, mdProvider.getMetadataProvider(), executorProvider);
         if (LOG.isDebugEnabled()) {
           LOG.debug("Plan after CTE rewriting:\n{}", RelOptUtil.toString(calcitePlan));

ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/CommonTableExpressionPrintSuggester.java

@@ -91,6 +91,7 @@
 import org.apache.hadoop.hive.common.ValidTxnWriteIdList;
 import org.apache.hadoop.hive.common.metrics.common.MetricsConstant;
 import org.apache.hadoop.hive.conf.Constants;
+import org.apache.hadoop.hive.conf.CteSuggesterType;
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
 import org.apache.hadoop.hive.conf.HiveConf.ResultFileFormat;
@@ -13064,7 +13065,7 @@ protected boolean analyzeAndResolveChildTree(ASTNode child, PlannerContext plann
 
     // Resolve Parse Tree and Get Metadata
     // Materialization is allowed if it is not a view definition
-    getMetaData(qb, createVwDesc == null && !forViewCreation);
+    getMetaData(qb, createVwDesc == null && !forViewCreation && CteSuggesterType.AST.enabled(conf));
     LOG.info("Completed getting MetaData in Semantic Analysis");
 
     return true;