wangzhigang1999 commented on code in PR #7419:
URL: https://github.com/apache/kyuubi/pull/7419#discussion_r3154977054
##########
externals/kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/operation/JdbcOperationManager.scala:
##########
@@ -91,7 +93,12 @@ class JdbcOperationManager(conf: KyuubiConf) extends
OperationManager("JdbcOpera
schemaName: String,
tableName: String,
tableTypes: util.List[String]): Operation = {
- val query = dialect.getTablesQuery(catalogName, schemaName, tableName,
tableTypes)
+ val effectiveSchema = if (StringUtils.isBlank(schemaName) || schemaName ==
"%") {
+ session.asInstanceOf[JdbcSessionImpl].effectiveDatabase.orNull
+ } else {
+ schemaName
+ }
+ val query = dialect.getTablesQuery(catalogName, effectiveSchema,
tableName, tableTypes)
Review Comment:
> > migrating the dialect API to call DatabaseMetaData directly will touch
every dialect, the operation manager, and every metadata test
>
> if we do that first, do we still need this patch?
Yes — the bug is the Hive JDBC client converting `null` `schemaPattern` to
`"%"`, and JDBC spec says `"%"` means "don't narrow by schema", so calling
`DatabaseMetaData.getTables(...)` directly reproduces it. Verified on
mysql-connector-j 8.4.0 (connection opened against `jdbc:mysql://.../dbA`):
```
conn.getCatalog() == "dbA"
getTables(null, "%", "%", null) → tables from dbA AND dbB ← bug reproduces
getTables("dbA", null, "%", null) → tables from dbA only ← only narrows
when explicit
```
So the engine still has to substitute the URL-scoped database into the call
when the client sends `null`/`"%"`. That routing logic (`effectiveDatabase` +
the if-blank-then-route block in `JdbcOperationManager`) survives the refactor
unchanged — only the line that builds a SQL string flips to
`conn.getMetaData.getTables(...)`. The `setSchema`-on-session-open machinery is
also independent.
What the refactor **does** subsume: the two `getCatalogsOperation` /
`getSchemasOperation` SQL implementations I added to `MySQLDialect` — those go
away once the engine calls `getCatalogs()` / `getSchemas()` directly.
Happy either way on ordering — bug fix first then refactor, or refactor
first and a smaller patch on top.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
