-
Notifications
You must be signed in to change notification settings - Fork 981
[KYUUBI #7305] Fix DBeaver connect returned all tables. return tables… #7307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…tables from current database now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue where DBeaver connecting to Kyuubi would return all tables from all databases instead of just the current database. The fix specifically addresses the behavior when the schema parameter is "%" by modifying the query to filter by the current database using the database() function.
Changes:
- Modified
MySQLDialect.getTablesQuery()to handle the "%" schema pattern by filtering to the current database - Added
getSchemasOperation()override toMySQLDialectto return the current database - Added
getCatalogsOperation()override toStarRocksDialectto use "show catalogs" command
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| MySQLDialect.scala | Added getSchemasOperation method and updated getTablesQuery to filter by current database when schema is "%" or blank |
| StarRocksDialect.scala | Added getCatalogsOperation method to return "show catalogs" command |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| override def getSchemasOperation(catalog: String, schema: String): String = { | ||
| return "select database()" |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explicit return statements are not idiomatic in Scala. Remove the 'return' keyword and let the expression be the implicit return value.
| return "select database()" | |
| "select database()" |
| override def getCatalogsOperation(): String = { | ||
| return "show catalogs" | ||
| } |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explicit return statements are not idiomatic in Scala. Remove the 'return' keyword and let the expression be the implicit return value.
| override def getCatalogsOperation(): String = { | |
| return "show catalogs" | |
| } | |
| override def getCatalogsOperation(): String = "show catalogs" |
|
|
||
| override def getSchemaHelper(): SchemaHelper = new StarRocksSchemaHelper | ||
|
|
||
|
|
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an unnecessary blank line before this method. Remove the extra blank line to maintain consistent spacing with other methods in the class.
| } | ||
|
|
||
| if (StringUtils.isNotBlank(schema)) { | ||
| // when DBeaver connect use %, kyuubi to return all tables from all other databases. |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected grammar in comment: 'use' should be 'uses', and the sentence structure should be improved for clarity. Suggested: 'when DBeaver connects using %, Kyuubi returns all tables from all databases.'
| // when DBeaver connect use %, kyuubi to return all tables from all other databases. | |
| // when DBeaver connects using %, Kyuubi returns all tables from all databases. |
| if (StringUtils.isNotBlank(schema) && !"%".equals(schema)) { | ||
| filters += s"$TABLE_SCHEMA LIKE '$schema'" | ||
| } else { | ||
| filters += s"$TABLE_SCHEMA = database()" |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition combines two separate cases: when schema is blank and when schema is '%'. This logic would be clearer if split into separate conditions to distinguish between 'schema is blank' and 'schema is %' cases. Consider restructuring as: if blank -> database(), else if '%' -> database(), else -> LIKE schema.
| if (StringUtils.isNotBlank(schema) && !"%".equals(schema)) { | |
| filters += s"$TABLE_SCHEMA LIKE '$schema'" | |
| } else { | |
| filters += s"$TABLE_SCHEMA = database()" | |
| if (StringUtils.isBlank(schema)) { | |
| // No schema provided: default to current database | |
| filters += s"$TABLE_SCHEMA = database()" | |
| } else if ("%".equals(schema)) { | |
| // '%' means all schemas: do not add TABLE_SCHEMA filter | |
| } else { | |
| filters += s"$TABLE_SCHEMA LIKE '$schema'" |
| } | ||
|
|
||
| override def getSchemasOperation(catalog: String, schema: String): String = { | ||
| return "select database()" |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method accepts catalog and schema parameters but doesn't use them. This implementation ignores the parameters entirely, which may not be the expected behavior for all use cases. Consider documenting this behavior or validating if the parameters should influence the query.
| return "select database()" | |
| // MySQLDialect currently returns only the current database as the schema. | |
| // The catalog and schema parameters are accepted for interface compatibility | |
| // but do not influence the generated query. | |
| if (StringUtils.isNotBlank(catalog) || StringUtils.isNotBlank(schema)) { | |
| // Parameters are intentionally ignored; behavior is to always return the current database. | |
| } | |
| "select database()" |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7307 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 698 698
Lines 43648 43648
Branches 5894 5894
======================================
Misses 43648 43648 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| if (StringUtils.isNotBlank(schema)) { | ||
| // when DBeaver connect use %, kyuubi to return all tables from all other databases. | ||
| if (StringUtils.isNotBlank(schema) && !"%".equals(schema)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this change, I suppose WHERE $TABLE_SCHEMA LIKE '%' has the same effect as an empty condition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
like MySQLDialect(mysql, doris, starrocks) use:
SELECT TABLE_CATALOG, TABLE_SCHEMA, TABLE_NAME, TABLE_TYPE, ENGINE,
TABLE_ROWS, AVG_ROW_LENGTH, DATA_LENGTH,
CREATE_TIME, UPDATE_TIME, TABLE_COLLATION, TABLE_COMMENT
FROM INFORMATION_SCHEMA.TABLES
当 Dbeaver 连接时,会采用 % 作为参数,这样或造成返回给 Dbeaver 所有库下的所有的表。
- 和session配置的DB 不符,用户看到了其他数据库下的表(所有表);
- Dbeaver 在执行查询时,schema 不对,查询报错;
When connecting with Dbeaver, it uses '%' as a parameter, which may cause Dbeaver to return tables from all databases.
- The database configuration may differ from the session's database, resulting in tables from other databases being displayed;
- Dbeaver may encounter query errors due to an incorrect schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can see issue: #7305
My StarRocks database "dataeye_starpony" only has 2 tables. However, DBeaver shows many more tables, and these extra tables cannot be queried.
return tables from current database now! doris/starrocks like mysql