This is an automated email from the ASF dual-hosted git repository.
jackie 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 89462abca2 Handle DUAL SQL Queries without NullPointerException in
JDBC (#13846)
89462abca2 is described below
commit 89462abca2987d0e09563c74dc9c5ffaaab22140
Author: Rajat Venkatesh <1638298+vra...@users.noreply.github.com>
AuthorDate: Tue Aug 20 01:55:51 2024 +0530
Handle DUAL SQL Queries without NullPointerException in JDBC (#13846)
---
.../java/org/apache/pinot/client/Connection.java | 11 +++++++---
.../pinot/common/utils/request/RequestUtils.java | 4 +++-
.../common/utils/request/RequestUtilsTest.java | 25 ++++++++++++++++++++++
.../tests/OfflineClusterIntegrationTest.java | 13 +++++++++++
4 files changed, 49 insertions(+), 4 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 99c2121e96..429c7be76a 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
@@ -21,6 +21,7 @@ package org.apache.pinot.client;
import java.util.Arrays;
import java.util.List;
import java.util.Properties;
+import java.util.Set;
import java.util.concurrent.CompletableFuture;
import javax.annotation.Nullable;
import org.apache.pinot.common.utils.request.RequestUtils;
@@ -96,7 +97,8 @@ public class Connection {
String[] tableNames = (tableName == null) ? resolveTableName(query) : new
String[]{tableName};
String brokerHostPort = _brokerSelector.selectBroker(tableNames);
if (brokerHostPort == null) {
- throw new PinotClientException("Could not find broker to query for
table(s): " + Arrays.asList(tableNames));
+ throw new PinotClientException("Could not find broker to query " +
((tableNames == null) ? "with no tables"
+ : "for table(s): " + Arrays.asList(tableNames)));
}
BrokerResponse response = _transport.executeQuery(brokerHostPort, query);
if (response.hasExceptions() && _failOnExceptions) {
@@ -143,8 +145,11 @@ public class Connection {
private static String[] resolveTableName(String query) {
try {
SqlNodeAndOptions sqlNodeAndOptions =
CalciteSqlParser.compileToSqlNodeAndOptions(query);
- return
RequestUtils.getTableNames(CalciteSqlParser.compileSqlNodeToPinotQuery(sqlNodeAndOptions.getSqlNode()))
- .toArray(new String[0]);
+ Set<String> tableNames =
+
RequestUtils.getTableNames(CalciteSqlParser.compileSqlNodeToPinotQuery(sqlNodeAndOptions.getSqlNode()));
+ if (tableNames != null) {
+ return tableNames.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 7cc8387731..e8feaeeb07 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
@@ -532,7 +532,9 @@ public class RequestUtils {
}
private static Set<String> getTableNames(DataSource dataSource) {
- if (dataSource.getSubquery() != null) {
+ if (dataSource == null) {
+ return null;
+ } else if (dataSource.getSubquery() != null) {
return getTableNames(dataSource.getSubquery());
} else if (dataSource.isSetJoin()) {
return
ImmutableSet.<String>builder().addAll(getTableNames(dataSource.getJoin().getLeft()))
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/utils/request/RequestUtilsTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/utils/request/RequestUtilsTest.java
index 65b0128497..bb14d048de 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/utils/request/RequestUtilsTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/utils/request/RequestUtilsTest.java
@@ -18,16 +18,20 @@
*/
package org.apache.pinot.common.utils.request;
+import java.util.Set;
import org.apache.calcite.sql.SqlDialect;
import org.apache.calcite.sql.SqlLiteral;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.pinot.common.request.Expression;
import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.sql.parsers.CalciteSqlParser;
import org.apache.pinot.sql.parsers.PinotSqlType;
import org.apache.pinot.sql.parsers.SqlNodeAndOptions;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
@@ -56,4 +60,25 @@ public class RequestUtilsTest {
assertEquals(result.getSqlNode().toSqlString((SqlDialect) null).toString(),
"SELECT `foo`\n" + "FROM `countries`\n" + "WHERE `bar` > 1");
}
+
+ @DataProvider(name = "queryProvider")
+ public Object[][] queryProvider() {
+ return new Object[][] {
+ {"select foo from countries where bar > 1", Set.of("countries")},
+ {"select 1", null}
+ };
+ }
+
+ @Test(dataProvider = "queryProvider")
+ public void testResolveTableNames(String query, Set<String> expectedSet) {
+ SqlNodeAndOptions sqlNodeAndOptions =
CalciteSqlParser.compileToSqlNodeAndOptions(query);
+ Set<String> tableNames =
+
RequestUtils.getTableNames(CalciteSqlParser.compileSqlNodeToPinotQuery(sqlNodeAndOptions.getSqlNode()));
+
+ if (expectedSet == null) {
+ assertNull(tableNames);
+ } else {
+ assertEquals(tableNames, expectedSet);
+ }
+ }
}
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
index 5d99bc0a36..63e9f80e79 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
@@ -3407,6 +3407,19 @@ public class OfflineClusterIntegrationTest extends
BaseClusterIntegrationTestSet
Assert.assertTrue(connection.isClosed());
}
+ @Test
+ public void testNoTableQueryThroughJDBCClient()
+ throws Exception {
+ String query = "SELECT 1";
+ java.sql.Connection connection =
getJDBCConnectionFromController(getControllerPort());
+ Statement statement = connection.createStatement();
+ ResultSet resultSet = statement.executeQuery(query);
+ resultSet.first();
+ Assert.assertTrue(resultSet.getLong(1) > 0);
+ connection.close();
+ Assert.assertTrue(connection.isClosed());
+ }
+
private java.sql.Connection getJDBCConnectionFromController(int
controllerPort)
throws Exception {
PinotDriver pinotDriver = new PinotDriver();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
