search

apache / kyuubi

Apache Kyuubi is a distributed and multi-tenant gateway to provide serverless SQL on data warehouses and lakehouses.
https://kyuubi.apache.org/
Apache License 2.0
2.1k stars 915 forks source link

[Bug] KyuubiStatement.execute always return true #2654

Open gabrywu opened 2 years ago

gabrywu commented 2 years ago

Code of Conduct

Search before asking

Describe the bug

KyuubiStatement.execute always return true even it's running a CREATE TABLE command in Spark SQL engine which doesn't return any results

true if the first result is a ResultSet object; false if it is an update count or there are no results boolean execute(String sql) throws SQLException;

Affects Version(s)

1.6-SNAPSHOT

Kyuubi Server Log Output

No response

Kyuubi Engine Log Output

No response

Kyuubi Server Configurations

No response

Kyuubi Engine Configurations

No response

Additional context

No response

Are you willing to submit PR?

pan3793 commented 2 years ago

Sounds like the condition check does not cover the case?

if (!status.isHasResultSet() && !stmtHandle.isHasResultSet()) {
  return false;
}

https://github.com/apache/incubator-kyuubi/blob/f16ac8be9fba238541b0a628a8fa8b4db47050bd/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiStatement.java#L242-L266

gabrywu commented 2 years ago

Sounds like the condition check does not cover the case?

if (!status.isHasResultSet() && !stmtHandle.isHasResultSet()) {
  return false;
}

https://github.com/apache/incubator-kyuubi/blob/f16ac8be9fba238541b0a628a8fa8b4db47050bd/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiStatement.java#L242-L266

stmtHandle.isHasResultSet() always return true

pan3793 commented 2 years ago

I see, we always do setHasResultSet(true) in engine side ExecuteStatement#beforeRun, it's not a client side problem.

gabrywu commented 2 years ago

is it possible to fix it?

gabrywu commented 2 years ago

In hiveserver2 built-in jdbc, it's determined by the SQL type. Such as, false is returned when DDL executes, true for QUERY executes

pan3793 commented 2 years ago

We have a KyuubiSqlClassification in kyuubi-extension-spark-3-1 module to classific statement kind, it's may increase maintainance effort if we move it to SQL engine because the commands change frequently across Spark version.

gabrywu commented 2 years ago

We have a KyuubiSqlClassification in kyuubi-extension-spark-3-1 module to classific statement kind, it's may increase maintainance effort if we move it to SQL engine because the commands change frequently across Spark version.

what about hive, presto, and other engines? How they determine the SQL type?