You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by xi...@apache.org on 2023/07/09 08:32:10 UTC

[pinot] branch master updated: Make RequestUtils always return a string array when getTableNames (#11069)

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

xiangfu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 12a090d90d Make RequestUtils always return a string array when getTableNames (#11069)
12a090d90d is described below

commit 12a090d90d3f339b7cec674e208dcda3b6b0bc8f
Author: Xiang Fu <xi...@gmail.com>
AuthorDate: Sun Jul 9 01:32:05 2023 -0700

    Make RequestUtils always return a string array when getTableNames (#11069)
---
 .../java/org/apache/pinot/client/Connection.java    |  5 ++---
 .../pinot/common/utils/request/RequestUtils.java    | 21 ++++++++++++++++-----
 .../api/resources/PinotQueryResource.java           |  6 +++---
 .../integration/tests/TPCHQueryIntegrationTest.java |  4 ----
 4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/Connection.java b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/Connection.java
index 49c12f2c8b..f43d465915 100644
--- a/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/Connection.java
+++ b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/Connection.java
@@ -191,9 +191,8 @@ public class Connection {
   private static String[] resolveTableName(String query) {
     try {
       SqlNodeAndOptions sqlNodeAndOptions = CalciteSqlParser.compileToSqlNodeAndOptions(query);
-      String tableName =
-          RequestUtils.getTableName(CalciteSqlParser.compileSqlNodeToPinotQuery(sqlNodeAndOptions.getSqlNode()));
-      return new String[]{tableName};
+      return RequestUtils.getTableNames(CalciteSqlParser.compileSqlNodeToPinotQuery(sqlNodeAndOptions.getSqlNode()))
+          .toArray(new String[0]);
     } catch (Exception e) {
       LOGGER.error("Cannot parse table name from query: {}. Fallback to broker selector default.", query, e);
     }
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
index 26f8cbc32a..6ac7845c9d 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
@@ -22,11 +22,14 @@ import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableSet;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Set;
 import org.apache.calcite.sql.SqlLiteral;
 import org.apache.calcite.sql.SqlNumericLiteral;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.pinot.common.request.DataSource;
 import org.apache.pinot.common.request.Expression;
 import org.apache.pinot.common.request.ExpressionType;
 import org.apache.pinot.common.request.Function;
@@ -51,7 +54,7 @@ public class RequestUtils {
   }
 
   public static SqlNodeAndOptions parseQuery(String query)
-          throws SqlCompilationException {
+      throws SqlCompilationException {
     return parseQuery(query, EMPTY_OBJECT_NODE);
   }
 
@@ -265,11 +268,19 @@ public class RequestUtils {
     return null;
   }
 
-  public static String getTableName(PinotQuery pinotQuery) {
-    while (pinotQuery.getDataSource().getSubquery() != null) {
-      pinotQuery = pinotQuery.getDataSource().getSubquery();
+  private static Set<String> getTableNames(DataSource dataSource) {
+    if (dataSource.getSubquery() != null) {
+      return getTableNames(dataSource.getSubquery());
+    } else if (dataSource.isSetJoin()) {
+      return ImmutableSet.<String>builder()
+          .addAll(getTableNames(dataSource.getJoin().getLeft()))
+          .addAll(getTableNames(dataSource.getJoin().getLeft())).build();
     }
-    return pinotQuery.getDataSource().getTableName();
+    return ImmutableSet.of(dataSource.getTableName());
+  }
+
+  public static Set<String> getTableNames(PinotQuery pinotQuery) {
+    return getTableNames(pinotQuery.getDataSource());
   }
 
   public static Map<String, String> getOptionsFromJson(JsonNode request, String optionsKey) {
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java
index fe1a217d9c..a8fb80f421 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java
@@ -239,8 +239,8 @@ public class PinotQueryResource {
     String tableName;
     try {
       String inputTableName =
-          sqlNode != null ? RequestUtils.getTableName(CalciteSqlParser.compileSqlNodeToPinotQuery(sqlNode))
-              : CalciteSqlCompiler.compileToBrokerRequest(query).getQuerySource().getTableName();
+          sqlNode != null ? RequestUtils.getTableNames(CalciteSqlParser.compileSqlNodeToPinotQuery(sqlNode)).iterator()
+              .next() : CalciteSqlCompiler.compileToBrokerRequest(query).getQuerySource().getTableName();
       tableName = _pinotHelixResourceManager.getActualTableName(inputTableName);
     } catch (Exception e) {
       LOGGER.error("Caught exception while compiling query: {}", query, e);
@@ -274,7 +274,7 @@ public class PinotQueryResource {
   // given a list of tables, returns the list of tableConfigs
   private List<TableConfig> getListTableConfigs(List<String> tableNames) {
     List<TableConfig> allTableConfigList = new ArrayList<>();
-    for (String tableName: tableNames) {
+    for (String tableName : tableNames) {
       List<TableConfig> tableConfigList = new ArrayList<>();
       if (_pinotHelixResourceManager.hasRealtimeTable(tableName)) {
         tableConfigList.add(Objects.requireNonNull(_pinotHelixResourceManager.getRealtimeTableConfig(tableName)));
diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TPCHQueryIntegrationTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TPCHQueryIntegrationTest.java
index cca6a0f740..a0cdb6e6bc 100644
--- a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TPCHQueryIntegrationTest.java
+++ b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TPCHQueryIntegrationTest.java
@@ -64,12 +64,8 @@ public class TPCHQueryIntegrationTest extends BaseClusterIntegrationTest {
     TPCH_QUICKSTART_TABLE_RESOURCES.put("part", "examples/batch/tpch/part");
     TPCH_QUICKSTART_TABLE_RESOURCES.put("supplier", "examples/batch/tpch/supplier");
     EXEMPT_QUERIES = new HashSet<>();
-    // The following query fails due to a bug in resolving table names from query
-    EXEMPT_QUERIES.add(13);
     // The following queries fail due to lack of support for views.
     EXEMPT_QUERIES.addAll(ImmutableList.of(15, 16, 17));
-    // The following query fails due to inability of Aggregation operator to handle conversion from Boolean to Number.
-    EXEMPT_QUERIES.add(23);
   }
 
   @BeforeClass


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org