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

chore: Validate table names locally first #1873

Merged
merged 3 commits into from
Oct 22, 2024
Merged

Conversation

msfstef
Copy link
Contributor

@msfstef msfstef commented Oct 21, 2024

Fixes #1814

We still go to PG to validate the table as we need its OID, so this is not strictly necessary, but since we have validation logic we might as well validate things locally to avoid going to PG for clearly invalid table identifiers?

This also fixes the accepted characters by PG for identifiers based on the docs as it was excluding some unicode chars and dollar signs before.

@balegas
Copy link
Contributor

balegas commented Oct 21, 2024

That is a cool little thing. Nice!

Copy link
Contributor

@kevin-dp kevin-dp left a comment

Choose a reason for hiding this comment

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

Nice work!
I left a few minor comments.

@msfstef msfstef merged commit a8b36ac into main Oct 22, 2024
24 checks passed
@msfstef msfstef deleted the msfstef/vaildate-table-name branch October 22, 2024 08:00
KyleAMathews pushed a commit that referenced this pull request Nov 1, 2024
Fixes #1814

We still go to PG to validate the table as we need its OID, so this is
not strictly necessary, but since we have validation logic we might as
well validate things locally to avoid going to PG for clearly invalid
table identifiers?

This also fixes the accepted characters by PG for identifiers [based on
the
docs](https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS)
as it was excluding some unicode chars and dollar signs before.
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.

Implement PG identifier parsing algorithm
4 participants