Skip to content

fix: use fully qualified name to resolve DuckDB ambiguous reference error#136

Closed
jerome-ng wants to merge 1 commit intoOHDSI:developfrom
jerome-ng:jerome-ng/1914-duckdb-ambiguous-schema-reference
Closed

fix: use fully qualified name to resolve DuckDB ambiguous reference error#136
jerome-ng wants to merge 1 commit intoOHDSI:developfrom
jerome-ng:jerome-ng/1914-duckdb-ambiguous-schema-reference

Conversation

@jerome-ng
Copy link
Copy Markdown
Contributor

@jerome-ng jerome-ng commented Mar 5, 2026

Copilot AI review requested due to automatic review settings March 5, 2026 08:51
@jerome-ng jerome-ng requested a review from p-hoffmann as a code owner March 5, 2026 08:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a DuckDB “Ambiguous reference to catalog or schema” error by changing how $$SCHEMA$$ placeholders are expanded in generated SQL within the TREX runtime.

Changes:

  • Expand $$SCHEMA$$ replacements from schema.table to catalog.schema.table by prepending the current database/catalog name.
  • Add inline documentation explaining the DuckDB ambiguity being addressed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +404 to +406
// "Ambiguous reference to catalog or schema" error when databaseCode equals schemaName
const databaseName = this.connection.getdatabase();
const replacement = schemaName === "" ? "" : `${databaseName}.${schemaName}.`;
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

This change always rewrites $$SCHEMA$$.* references to a 3-part name (catalog.schema.table), but the comment/PR context suggests it’s only required when the catalog name and schema name collide (e.g., databaseCode === schemaName). To reduce the blast radius of SQL generation changes, consider only prefixing with ${databaseName}. when schemaName !== "" and schemaName === databaseName (or otherwise detecting the ambiguous case), and keep the prior ${schemaName}. behavior for the common case.

Suggested change
// "Ambiguous reference to catalog or schema" error when databaseCode equals schemaName
const databaseName = this.connection.getdatabase();
const replacement = schemaName === "" ? "" : `${databaseName}.${schemaName}.`;
// "Ambiguous reference to catalog or schema" error when databaseCode equals schemaName.
// To reduce blast radius, only include the catalog when the schema name collides with it.
const databaseName = this.connection.getdatabase();
let replacement = "";
if (schemaName !== "") {
if (schemaName === databaseName) {
replacement = `${databaseName}.${schemaName}.`;
} else {
replacement = `${schemaName}.`;
}
}

Copilot uses AI. Check for mistakes.
@p-hoffmann p-hoffmann closed this Mar 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cohort page throws error when databaseCode and SchemaName are same

3 participants