zabetak commented on code in PR #6673:
URL: https://github.com/apache/hive/pull/6673#discussion_r4023631059


##########
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java:
##########
@@ -202,7 +203,64 @@ public MiniLlapLocalCliConfig() {
       }
     }
   }
-  
+
+  public static class MiniLlapLocalPostgresJdbcCliConfig extends 
AbstractCliConfig {
+    private final QTestDatabaseHandler.DatabaseType databaseType;
+    private final String jdbcInitScript;
+    private final String externalTablesInitScript;
+
+    public MiniLlapLocalPostgresJdbcCliConfig() {
+      super(CoreJdbcCliDriver.class);
+      try {
+        databaseType = QTestDatabaseHandler.DatabaseType.POSTGRES;
+        jdbcInitScript = "q_test_tpcds_schema.postgres.sql";
+        externalTablesInitScript = 
"q_test_tpcds_external_tables_schema.postgres.sql";
+
+        setQueryDir("ql/src/test/queries/clientpositive/perf");
+        
setLogDir("itests/qtest/target/qfile-results/clientpositive/jdbc/postgres");
+        setResultsDir("ql/src/test/results/clientpositive/jdbc/postgres");
+        setHiveConfDir("data/conf/llap");
+        setClusterType(MiniClusterType.LLAP_LOCAL);
+        setCustomConfigValueMap(createConfVarsStringMap());
+        excludesFrom(testConfigProps, "jdbc.disabled.query.files");
+        // Run CBO plans only
+        includeCboQueryFiles();
+      } catch (Exception e) {
+        throw new RuntimeException("can't construct cliconfig", e);
+      }
+    }
+
+    private void includeCboQueryFiles() {
+      File[] cboQueryFiles = new File(getQueryDirectory())
+          .listFiles((dir, name) -> name.startsWith("cbo") && 
name.endsWith(".q"));
+      if (cboQueryFiles != null) {
+        for (File f : cboQueryFiles) {
+          includeQuery(f.getName());
+        }
+      }
+    }
+
+    private static Map<HiveConf.ConfVars, String> createConfVarsStringMap() {
+      Map<HiveConf.ConfVars, String> conf = new HashMap<>();

Review Comment:
   EnumMap would be a better choice as Sonar highlights but rather minor point.



##########
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreJdbcCliDriver.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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

Review Comment:
   Not sure why the license line is flagged by Sonar. Bad copy paste?



##########
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreJdbcCliDriver.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.cli.control;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hive.ql.externalDB.AbstractExternalDB;
+import org.apache.hadoop.hive.ql.QTestUtil;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+public class CoreJdbcCliDriver extends CoreCliDriver {

Review Comment:
   Maybe you can make use of the `CorePerfCliDriver` that is also made 
especially for skipping `clearTestSideEffects`.
   
   The database start/stop logic would fit better inside `QTestUtil` where we 
already create `QTestDatabaseHandler`. Basically, I am trying to see if there 
is a way to generalize a bit to make the logic usable by other `CliConfigs`.
   
   Anyways this falls into the category of refactoring so I am ok to merge this 
as is and log follow-up ticket to make this a bit more general.



##########
ql/src/test/results/clientpositive/jdbc/postgres/query1.q.out:
##########
@@ -0,0 +1,55 @@
+STAGE DEPENDENCIES:
+  Stage-0 is a root stage
+
+STAGE PLANS:
+  Stage: Stage-0
+    Fetch Operator
+      limit: -1
+      Processor Tree:
+        TableScan
+          alias: store_returns
+          properties:
+            hive.sql.query SELECT "t25"."c_customer_id"

Review Comment:
   The files were removed so marking this conversation as resolved.



##########
ql/src/test/results/clientpositive/jdbc/postgres/cbo_query12.q.out:
##########
@@ -0,0 +1,22 @@
+CBO PLAN:
+HiveProject(i_item_desc=[$0], i_category=[$1], i_class=[$2], 
i_current_price=[$3], itemrevenue=[$4], revenueratio=[$5])
+  HiveSortLimit(sort0=[$1], sort1=[$2], sort2=[$6], sort3=[$0], sort4=[$5], 
dir0=[ASC], dir1=[ASC], dir2=[ASC], dir3=[ASC], dir4=[ASC], fetch=[100])

Review Comment:
   Databases support window functions so maybe we shouldn't block all pushdowns 
but only those not supported by the underlying engine. Let's log a follow-up if 
that makes sense.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to