Skip to content

Fixed the problem that importing the specified database name does not take effect. #1930

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fixed the problem that importing the specified database name does not take effect. #1930

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
972415b
Fixed the problem that importing the specified database name does not…
tonyeatsm Sep 2, 2025
dff3185
Merge branch 'main' into wrenai-dev
tonyeatsm Sep 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion wren-ui/src/apollo/server/services/metadataService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,41 @@ export class DataSourceMetadataService implements IDataSourceMetadataService {
const tables = await this.wrenEngineAdaptor.listTables();
return tables;
}
return await this.ibisAdaptor.getTables(dataSource, connectionInfo);

const allTables = await this.ibisAdaptor.getTables(
dataSource,
connectionInfo,
);

// Filter tables by database/schema if database is specified in connectionInfo
const database = connectionInfo?.database;
if (database && allTables.length > 0) {
// For data sources that use database as schema name, filter tables by schema
return allTables.filter((table) => {
// Check if table name contains schema information
// Table names might be in format: schema.table or just table
const tableName = table.name;
const tableNameParts = tableName.split('.');

if (tableNameParts.length > 1) {
// If table name has schema prefix, check if it matches the database
const schemaName = tableNameParts[0];
return schemaName === database;
} else {
// If no schema prefix, check table properties for schema information
const tableSchema =
table.properties?.schema || table.properties?.database_name;
if (tableSchema) {
return tableSchema === database;
}
// If no schema information available, include the table
// (this maintains backward compatibility)
return true;
}
});
}
Comment on lines +81 to +107
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Normalize identifiers and handle multi-part names (e.g., BigQuery) to avoid false negatives.

Current logic assumes the schema is the first segment before a dot. For BigQuery, fully-qualified names can be project.dataset.table, where the dataset is the penultimate segment. Also, comparisons are case/quote sensitive, and properties may expose dataset/schema under different keys. Suggest normalized, data-source-aware matching with broader property fallback.

Apply this diff within the current block to make matching robust while preserving your backward-compat behavior:

-    // Filter tables by database/schema if database is specified in connectionInfo
-    const database = connectionInfo?.database;
-    if (database && allTables.length > 0) {
-      // For data sources that use database as schema name, filter tables by schema
-      return allTables.filter((table) => {
-        // Check if table name contains schema information
-        // Table names might be in format: schema.table or just table
-        const tableName = table.name;
-        const tableNameParts = tableName.split('.');
-
-        if (tableNameParts.length > 1) {
-          // If table name has schema prefix, check if it matches the database
-          const schemaName = tableNameParts[0];
-          return schemaName === database;
-        } else {
-          // If no schema prefix, check table properties for schema information
-          const tableSchema =
-            table.properties?.schema || table.properties?.database_name;
-          if (tableSchema) {
-            return tableSchema === database;
-          }
-          // If no schema information available, include the table
-          // (this maintains backward compatibility)
-          return true;
-        }
-      });
-    }
+    // Filter tables by database/schema if database is specified in connectionInfo
+    const database =
+      connectionInfo?.database ??
+      connectionInfo?.schema ??
+      connectionInfo?.dataset;
+
+    // helper: strip quotes/backticks/brackets, lowercase, trim
+    const normalizeIdent = (v?: string) =>
+      typeof v === 'string'
+        ? v.replace(/[`"'[\]]/g, '').trim().toLowerCase()
+        : v;
+
+    if (database && Array.isArray(allTables) && allTables.length > 0) {
+      const dbNorm = normalizeIdent(database);
+      return allTables.filter((table) => {
+        // Prefer explicit properties if present
+        const propSchemaRaw =
+          table.properties?.schema ??
+          table.properties?.schema_name ??
+          table.properties?.database_name ??
+          table.properties?.database ??
+          table.properties?.dataset ??
+          table.properties?.dataset_name;
+        const propSchema = normalizeIdent(propSchemaRaw);
+        if (propSchema) {
+          return propSchema === dbNorm;
+        }
+
+        // Fallback: parse from table.name
+        const parts = String(table.name ?? '')
+          .replace(/[`"']/g, '')
+          .split('.')
+          .filter(Boolean);
+
+        // Data-source-aware extraction of schema/dataset segment
+        let schemaFromName: string | undefined;
+        if (dataSource === DataSourceName.BIGQUERY) {
+          // project.dataset.table -> dataset is the penultimate segment
+          schemaFromName = parts.length >= 2 ? parts[parts.length - 2] : undefined;
+        } else {
+          // postgres etc.: schema.table -> schema is first segment
+          schemaFromName = parts.length >= 2 ? parts[0] : undefined;
+        }
+        if (schemaFromName) {
+          return normalizeIdent(schemaFromName) === dbNorm;
+        }
+
+        // If no schema information available, include the table
+        // (maintains backward compatibility)
+        return true;
+      });
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Filter tables by database/schema if database is specified in connectionInfo
const database = connectionInfo?.database;
if (database && allTables.length > 0) {
// For data sources that use database as schema name, filter tables by schema
return allTables.filter((table) => {
// Check if table name contains schema information
// Table names might be in format: schema.table or just table
const tableName = table.name;
const tableNameParts = tableName.split('.');
if (tableNameParts.length > 1) {
// If table name has schema prefix, check if it matches the database
const schemaName = tableNameParts[0];
return schemaName === database;
} else {
// If no schema prefix, check table properties for schema information
const tableSchema =
table.properties?.schema || table.properties?.database_name;
if (tableSchema) {
return tableSchema === database;
}
// If no schema information available, include the table
// (this maintains backward compatibility)
return true;
}
});
}
// Filter tables by database/schema if database is specified in connectionInfo
const database =
connectionInfo?.database ??
connectionInfo?.schema ??
connectionInfo?.dataset;
// helper: strip quotes/backticks/brackets, lowercase, trim
const normalizeIdent = (v?: string) =>
typeof v === 'string'
? v.replace(/[`"'[\]]/g, '').trim().toLowerCase()
: v;
if (database && Array.isArray(allTables) && allTables.length > 0) {
const dbNorm = normalizeIdent(database);
return allTables.filter((table) => {
// Prefer explicit properties if present
const propSchemaRaw =
table.properties?.schema ??
table.properties?.schema_name ??
table.properties?.database_name ??
table.properties?.database ??
table.properties?.dataset ??
table.properties?.dataset_name;
const propSchema = normalizeIdent(propSchemaRaw);
if (propSchema) {
return propSchema === dbNorm;
}
// Fallback: parse from table.name
const parts = String(table.name ?? '')
.replace(/[`"']/g, '')
.split('.')
.filter(Boolean);
// Data-source-aware extraction of schema/dataset segment
let schemaFromName: string | undefined;
if (dataSource === DataSourceName.BIGQUERY) {
// project.dataset.table -> dataset is the penultimate segment
schemaFromName = parts.length >= 2 ? parts[parts.length - 2] : undefined;
} else {
// postgres etc.: schema.table -> schema is first segment
schemaFromName = parts.length >= 2 ? parts[0] : undefined;
}
if (schemaFromName) {
return normalizeIdent(schemaFromName) === dbNorm;
}
// If no schema information available, include the table
// (maintains backward compatibility)
return true;
});
}
🤖 Prompt for AI Agents 
In wren-ui/src/apollo/server/services/metadataService.ts around lines 81 to 107,
the current filter assumes the schema is the first dot-segment and does
case/quote-sensitive exact matches which fails for multi-part names (e.g.,
project.dataset.table), quoted identifiers, and varying property keys; update
the filter to: split name into segments and treat the penultimate segment as
schema for names with 3+ parts (first segment for 2-part names), normalize
identifiers by trimming surrounding quotes and lowercasing both table-derived
segments and the connectionInfo.database, check multiple property keys for
schema/dataset (e.g., schema, database_name, dataset, dataset_id, datasetId),
compare using normalized values, and preserve the backward-compat behavior of
including tables when no schema info is available.


return allTables;
}

public async listConstraints(
Expand Down