github-actions[bot] commented on code in PR #68255:
URL: https://github.com/apache/doris/pull/68255#discussion_r4061832032
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java:
##########
@@ -152,6 +153,7 @@ public class JdbcResource extends Resource {
OPTIONAL_PROPERTIES_DEFAULT_VALUE.put(ExternalCatalog.META_NAMES_MAPPING, "");
OPTIONAL_PROPERTIES_DEFAULT_VALUE.put(ExternalCatalog.INCLUDE_DATABASE_LIST,
"");
OPTIONAL_PROPERTIES_DEFAULT_VALUE.put(ExternalCatalog.EXCLUDE_DATABASE_LIST,
"");
+
OPTIONAL_PROPERTIES_DEFAULT_VALUE.put(ExternalCatalog.INCLUDE_INTERNAL_DATABASE_LIST,
"");
Review Comment:
[P2] Backfill this default for image-loaded resources
`applyDefaultProperties()` runs for new resources and edit-log CREATE
replay, but an older resource loaded through `ResourceMgr.read()` only executes
the inherited `Resource.gsonPostProcess()`, which does not add optional JDBC
defaults. Because `SHOW RESOURCES` iterates the stored `configs`, an old-image
JDBC resource will keep 22 rows while a newly created/replayed one now has the
asserted 23. Please backfill JDBC defaults during image deserialization (and
add an old-image compatibility test) so upgrade behavior is deterministic.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1181,6 +1182,10 @@ protected Map<String, Boolean> getExcludeDatabaseMap() {
return getSpecifiedDatabaseMap(ExternalCatalog.EXCLUDE_DATABASE_LIST);
}
+ protected Map<String, Boolean> getIncludeInternalDatabaseMap() {
Review Comment:
[P1] Handle reserved database names for explicit internal opt-ins
The JDBC filter sets include `mysql` and `information_schema`, so this
property appears to allow both. Downstream, however, `ExternalCatalog`
canonicalizes those reserved names and `buildDbForInit()` creates Doris's
synthetic `ExternalMysqlDatabase`/`ExternalInfoSchemaDatabase`; their table
lists never call JDBC metadata. Consequently
`include_internal_database_list=mysql` by itself still cannot expose the remote
`mysql` schema (an additional `meta_names_mapping` alias is currently
required). Please either provide a nonreserved mapping
automatically/documentedly for these opt-ins or reject the unsupported values
instead of silently returning a different database.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java:
##########
@@ -518,7 +534,9 @@ protected List<String> filterDatabaseNames(List<String>
remoteDbNames) {
continue;
}
}
- if (filterInternalDatabases.contains(databaseName.toLowerCase())) {
+ String normalizedDatabaseName =
databaseName.toLowerCase(Locale.ROOT);
+ if (filterInternalDatabases.contains(normalizedDatabaseName)
Review Comment:
[P1] Make internal opt-ins participate in database selection
When `only_specified_database=true`, the database-list implementations
return only the current schema whenever the ordinary include/exclude maps are
empty, without checking `includeInternalDatabaseMap`. If an ordinary include
list is present, lines 529-535 can reject the internal database before this
exemption runs. Thus `include_internal_database_list=performance_schema` is
silently ignored in the first case, and must be duplicated (case-sensitively)
into `include_database_list` in the second. Please make a nonempty internal
opt-in force enumeration and participate as an explicit allowance (while
preserving intentional exclude precedence), and cover these combinations.
--
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]
