Skip to content
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

Check SqlState instead of error message #161

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

Conversation

e-tobi
Copy link

@e-tobi e-tobi commented Dec 26, 2024

The PostgreSQL error messages are translated, so trying to match them against an english text might not work on different locales.

Please note, that here a 3 more places where the message text is evaluated, two of them only catching an Exception from reader.CloseAsync(). These lines of code are not not covered by any unit test and I couldn't figure out a sane way to trigger an exception there, so I left them untouched.

Copy link
Member

@mysticmind mysticmind left a comment

Choose a reason for hiding this comment

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

Just to be safe, I would suggest to change this to e.Message.Contains("does not exist") || e.SqlState == "3F000"?

@e-tobi
Copy link
Author

e-tobi commented Dec 27, 2024

There are A LOT of messages in PostgreSQL containing "does not exist":

https://github.com/postgres/postgres/blob/master/src/backend/po/de.po

I don't know which ones would make it to an exception, but my guess is "does not exist" is too unspecific.
Best solution would probably be to not rely on an exception at all and explicitly check for the schema existence.

Tobias Grimm added 2 commits December 30, 2024 22:24
The PostgreSQL error messages are translated, so trying to match them against an english text might not work on different locales.
Instead of checking for the exception message containing "does not exist", check if SqlState matches PostgresErrorCodes.UndefinedTable ("42P01").
@mysticmind
Copy link
Member

mysticmind commented Dec 31, 2024

@e-tobi I saw the changes what you have done to use PostgresErrorCodes, looks good to me as a fix. I have kick started the CI runs.

@mysticmind mysticmind self-requested a review December 31, 2024 18:01
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.

2 participants