- Notifications
You must be signed in to change notification settings - Fork2.7k
Improve catalog error message for missing schemas#19922
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
When querying a table in a non-existent schema, the error messagenow correctly indicates that the schema does not exist, rather thansaying the table does not exist.The new error message format is:'Catalog with name <schema> does not exist and there is no <schema> schema in the current catalog!'This provides clearer feedback to users when they reference a schemathat doesn't exist, making it easier to identify and fix the issue.Changes:- Added logic in Catalog::TryLookupEntry to detect when a schema doesn't exist and throw an appropriate error- Simplified code by removing redundant checks- Added comprehensive tests for various scenarios including: - Non-existent schema without catalog specification - Non-existent schema with catalog specification - Non-existent table in existing schema (still shows table error) - Multiple catalog scenarios
- Add check to only apply schema error logic when looking for TABLE_ENTRY- This prevents incorrect schema errors for column.method calls like filename.replace()- Update test expectations to match new accurate error messages- All tests pass including dot_function_missing_error.test
pdet left a comment
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.
Thanks for the PR, i had a small comment about it.
src/catalog/catalog.cpp Outdated
| EntryLookupInfoschema_lookup(CatalogType::SCHEMA_ENTRY, schema_name, lookup_info.GetErrorContext()); | ||
| auto except =CatalogException( | ||
| schema_lookup.GetErrorContext(), | ||
| "Catalog with name %s does not exist and there is no %s schema in the current catalog!", schema_name, | ||
| schema_name); |
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.
Wouldn't "Schema with name %s does not exist!" be a more clear error message?
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 other option would be to have an error message closer to what postgres does:
ERROR: relation "s.t1" does not existLINE 1: select * from s.t1There 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.
@taniabogatsch WDYT? What should the error message be?
I think it’s a good idea to have a message similar to Postgres.
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.
Yes, I like that, too - let's go with Postgres similarity. 👍
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.
@taniabogatsch@pdet done
Updated error message from 'Catalog with name X does not exist and there is no X schema in the current catalog!' to 'the relation "schema.table" does not exist' format. This provides clearer error messages showing the full qualified relation name.
- Updated catalog.cpp to generate error message in format 'the relation "schema.table" does not exist'- Updated all test files to expect the new error message format- Fixed remaining occurrences in test_schema.test and test_schema_dependency.test
| EntryLookupInfoschema_lookup(CatalogType::SCHEMA_ENTRY, schema_name, lookup_info.GetErrorContext()); | ||
| string relation_name = schema_name +"." + lookup_info.GetEntryName(); | ||
| auto except = | ||
| CatalogException(schema_lookup.GetErrorContext(),"the relation\"%s\" does not exist", relation_name); |
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.
Let's stick to "Table with name" for consistency with the other error message
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.
But that caused the confusion in the first place, no? Since it was not actually the table that was missing?
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 table is missing, otherwise the look-up wouldn't fail. The confusion is that the error doesn't happen in the look-up of the table name, but in the look-up of the schema, which is not included as part of the error message.
Note that "relation" is just how Postgres calls tables.
taniabogatschNov 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
So to confirm: looking at, e.g., this change. We want to stay with "table" and we want the error message to say:Catalog Error: Table with name "s1.test does not exist`?
taniabogatschNov 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Isn't that still misleading because, like you said, it happens during schema lookup? Shouldn't we at least add something likebecause schema "s1" does not exist?
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'm fine with that, but that is a more involved change in this PR. My comment here was to basically say "I don't see a reason to switch nomenclature from "Table" to "relation" for this error message"
Uh oh!
There was an error while loading.Please reload this page.
This PR improves catalog error messages for missing schemas.
Changes