From 856338b645a28ed4ee67b865e915a991645604d2 Mon Sep 17 00:00:00 2001 From: Tigran Manasyan Date: Wed, 28 Feb 2024 18:21:23 +0400 Subject: [PATCH] fix review comments --- .../engine/jdbc/dialect/ImpalaDialect.scala | 4 + .../src/test/resources/impala-compose.yml | 4 +- .../test/resources/impala_conf/hive-site.xml | 2 +- .../engine/jdbc/impala/DialectSuite.scala | 107 ++++++++++++++++++ .../jdbc/impala/WithImpalaContainer.scala | 2 +- .../src/test/resources/impala-compose.yml | 4 +- .../test/resources/impala_conf/hive-site.xml | 2 +- 7 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 externals/kyuubi-jdbc-engine/src/test/scala/org/apache/kyuubi/engine/jdbc/impala/DialectSuite.scala diff --git a/externals/kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/dialect/ImpalaDialect.scala b/externals/kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/dialect/ImpalaDialect.scala index 980baebdc55..2c08655ad06 100644 --- a/externals/kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/dialect/ImpalaDialect.scala +++ b/externals/kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/dialect/ImpalaDialect.scala @@ -55,6 +55,10 @@ class ImpalaDialect extends JdbcDialect { schemaName: String, tableName: String, columnName: String): String = { + if (StringUtils.isEmpty(tableName)) { + throw KyuubiSQLException("Table name should not be empty") + } + if (isPattern(schemaName)) { throw KyuubiSQLException.featureNotSupported("Pattern-like schema names not supported") } diff --git a/externals/kyuubi-jdbc-engine/src/test/resources/impala-compose.yml b/externals/kyuubi-jdbc-engine/src/test/resources/impala-compose.yml index 34f55882704..eb95cbef734 100644 --- a/externals/kyuubi-jdbc-engine/src/test/resources/impala-compose.yml +++ b/externals/kyuubi-jdbc-engine/src/test/resources/impala-compose.yml @@ -16,7 +16,7 @@ # under the License. # modified compose file from Impala repo, see -# https://github.com/apache/impala/blob/master/docker/quickstart.yml +# https://github.com/apache/impala/blob/4.3.0/docker/quickstart.yml version: "3.5" services: metastore: @@ -66,7 +66,7 @@ services: "-redirect_stdout_stderr=false", "-logtostderr", "-mt_dop_auto_fallback=true", "-default_query_options=mt_dop=4,default_file_format=parquet,default_transactional_type=insert_only", - "-mem_limit=4gb"] + "-mem_limit=1500mb"] environment: # Keep the Java heap small to preserve memory for query execution. - JAVA_TOOL_OPTIONS="-Xmx1g" diff --git a/externals/kyuubi-jdbc-engine/src/test/resources/impala_conf/hive-site.xml b/externals/kyuubi-jdbc-engine/src/test/resources/impala_conf/hive-site.xml index 97dbc022118..cc58f1ae15a 100644 --- a/externals/kyuubi-jdbc-engine/src/test/resources/impala_conf/hive-site.xml +++ b/externals/kyuubi-jdbc-engine/src/test/resources/impala_conf/hive-site.xml @@ -17,7 +17,7 @@ limitations under the License. --> diff --git a/externals/kyuubi-jdbc-engine/src/test/scala/org/apache/kyuubi/engine/jdbc/impala/DialectSuite.scala b/externals/kyuubi-jdbc-engine/src/test/scala/org/apache/kyuubi/engine/jdbc/impala/DialectSuite.scala new file mode 100644 index 00000000000..aef1a2d2143 --- /dev/null +++ b/externals/kyuubi-jdbc-engine/src/test/scala/org/apache/kyuubi/engine/jdbc/impala/DialectSuite.scala @@ -0,0 +1,107 @@ +/* + * 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.kyuubi.engine.jdbc.impala + +import org.apache.kyuubi.{KyuubiFunSuite, KyuubiSQLException} +import org.apache.kyuubi.engine.jdbc.dialect.ImpalaDialect +import org.apache.kyuubi.engine.jdbc.impala.DialectSuite.{SCHEME_NAME, TABLE_NAME} + +class DialectSuite extends KyuubiFunSuite { + + private val dialect: ImpalaDialect = new ImpalaDialect() + + test("impala dialect - get tables query") { + val expectedQuery = "show tables" + val actualQuery = dialect.getTablesQuery(null, null, null, null) + + assert(expectedQuery == actualQuery.trim) + } + + test("impala dialect - get tables query by scheme") { + val expectedQuery = s"show tables in $SCHEME_NAME" + val actualQuery = dialect.getTablesQuery(null, SCHEME_NAME, null, null) + + assert(expectedQuery == actualQuery.trim) + } + + test("impala dialect - get tables query by table name") { + val expectedQuery = s"show tables like '$TABLE_NAME'" + val queryWithTableName = dialect.getTablesQuery(null, null, TABLE_NAME, null) + + assert(expectedQuery == queryWithTableName.trim) + + // kyuubi injects '%' in case if schema is null + val queryWithWildcardScheme = dialect.getTablesQuery(null, "%", TABLE_NAME, null) + + assert(expectedQuery == queryWithWildcardScheme.trim) + } + + test("impala dialect - get tables query by scheme and table name") { + val expectedQuery = s"show tables in $SCHEME_NAME like 'test*'" + val actualQuery = dialect.getTablesQuery(null, SCHEME_NAME, "test*", null) + + assert(expectedQuery == actualQuery.trim) + } + + test("impala dialect - fail get tables if pattern-like scheme provided") { + val exception = intercept[KyuubiSQLException] { + dialect.getTablesQuery(null, "*scheme*", TABLE_NAME, null) + } + assert(exception.getMessage == "Pattern-like schema names not supported") + } + + test("impala dialect - get columns query by table name") { + val expectedQuery = s"show column stats $TABLE_NAME" + val actualQuery = dialect.getColumnsQuery(null, null, null, TABLE_NAME, null) + + assert(expectedQuery == actualQuery.trim) + } + + test("impala dialect - get columns query by scheme and table name") { + val expectedQuery = s"show column stats $SCHEME_NAME.$TABLE_NAME" + val actualQuery = dialect.getColumnsQuery(null, null, SCHEME_NAME, TABLE_NAME, null) + + assert(expectedQuery == actualQuery.trim) + } + + test("impala dialect - fail get columns if pattern-like scheme provided") { + val exception = intercept[KyuubiSQLException] { + dialect.getColumnsQuery(null, null, "*scheme*", TABLE_NAME, null) + } + assert(exception.getMessage == "Pattern-like schema names not supported") + } + + test("impala dialect - fail get columns if pattern-like table provided") { + val exception = intercept[KyuubiSQLException] { + dialect.getColumnsQuery(null, null, null, "*test*", null) + } + assert(exception.getMessage == "Pattern-like table names not supported") + } + + test("impala dialect - fail get columns if table name not provided") { + val exception = intercept[KyuubiSQLException] { + dialect.getColumnsQuery(null, null, null, null, null) + } + assert(exception.getMessage == "Table name should not be empty") + } +} + +object DialectSuite { + val TABLE_NAME = "test_table" + val SCHEME_NAME = "test_db" +} + diff --git a/externals/kyuubi-jdbc-engine/src/test/scala/org/apache/kyuubi/engine/jdbc/impala/WithImpalaContainer.scala b/externals/kyuubi-jdbc-engine/src/test/scala/org/apache/kyuubi/engine/jdbc/impala/WithImpalaContainer.scala index 9c41c60722a..4aaa8871141 100644 --- a/externals/kyuubi-jdbc-engine/src/test/scala/org/apache/kyuubi/engine/jdbc/impala/WithImpalaContainer.scala +++ b/externals/kyuubi-jdbc-engine/src/test/scala/org/apache/kyuubi/engine/jdbc/impala/WithImpalaContainer.scala @@ -63,6 +63,6 @@ trait WithImpalaContainer extends WithJdbcServerContainer { protected def hiveServerJdbcUrl: String = withContainers { container => val feHost: String = container.getServiceHost(IMPALAD_SERVICE_NAME, IMPALAD_PORT) val fePort: Int = container.getServicePort(IMPALAD_SERVICE_NAME, IMPALAD_PORT) - s"jdbc:hive2://$feHost:$fePort/;auth=noSasl" + s"jdbc:kyuubi://$feHost:$fePort/;auth=noSasl" } } diff --git a/integration-tests/kyuubi-jdbc-it/src/test/resources/impala-compose.yml b/integration-tests/kyuubi-jdbc-it/src/test/resources/impala-compose.yml index 34f55882704..eb95cbef734 100644 --- a/integration-tests/kyuubi-jdbc-it/src/test/resources/impala-compose.yml +++ b/integration-tests/kyuubi-jdbc-it/src/test/resources/impala-compose.yml @@ -16,7 +16,7 @@ # under the License. # modified compose file from Impala repo, see -# https://github.com/apache/impala/blob/master/docker/quickstart.yml +# https://github.com/apache/impala/blob/4.3.0/docker/quickstart.yml version: "3.5" services: metastore: @@ -66,7 +66,7 @@ services: "-redirect_stdout_stderr=false", "-logtostderr", "-mt_dop_auto_fallback=true", "-default_query_options=mt_dop=4,default_file_format=parquet,default_transactional_type=insert_only", - "-mem_limit=4gb"] + "-mem_limit=1500mb"] environment: # Keep the Java heap small to preserve memory for query execution. - JAVA_TOOL_OPTIONS="-Xmx1g" diff --git a/integration-tests/kyuubi-jdbc-it/src/test/resources/impala_conf/hive-site.xml b/integration-tests/kyuubi-jdbc-it/src/test/resources/impala_conf/hive-site.xml index 97dbc022118..cc58f1ae15a 100644 --- a/integration-tests/kyuubi-jdbc-it/src/test/resources/impala_conf/hive-site.xml +++ b/integration-tests/kyuubi-jdbc-it/src/test/resources/impala_conf/hive-site.xml @@ -17,7 +17,7 @@ limitations under the License. -->