Skip to content

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tigrulya-exe committed Feb 29, 2024
1 parent ecb0d7d commit 9852125
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
limitations under the License.
-->
<!--
Hive configuration for Impala quickstart docker cluster.
Hive configuration for Impala quickstart docker cluster.
-->
<configuration>
<property>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* 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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
limitations under the License.
-->
<!--
Hive configuration for Impala quickstart docker cluster.
Hive configuration for Impala quickstart docker cluster.
-->
<configuration>
<property>
Expand Down

0 comments on commit 9852125

Please sign in to comment.