Allow escaped quotes in Postgres quoted identifier#13179
Allow escaped quotes in Postgres quoted identifier#13179austinpgraham wants to merge 3 commits intosqlalchemy:mainfrom
Conversation
af491e6 to
6b01f16
Compare
|
thanks this looks great. |
sqla-tester
left a comment
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 6b01f16 of this pull request into gerrit so we can run tests and reviews and stuff
|
New Gerrit review created for change 6b01f16: https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6620 |
|
OK so for the pep8 thing, if you install pre-commit and then do a commit --amend, it will run all the required linters including black for code formatting. |
565105b to
6e5a159
Compare
6e5a159 to
8890dc3
Compare
|
@zzzeek Done, ran the pep8 test locall and now passes |
sqla-tester
left a comment
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 8890dc3 of this pull request into gerrit so we can run tests and reviews and stuff
|
Patchset 8890dc3 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6620 |
|
I'll add a changelog and make a backport. thanks for the pr @austinpgraham, no further action is required from your part, thanks |
|
Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6620 has been merged. Congratulations! :) |
|
Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6625 has been merged. Congratulations! :) |
<!-- Provide a general summary of your proposed changes in the Title field above --> ### Description Issue: #10902 Hello! This is my first PR here, so please let me know what I may have missed in terms of having a valuable contribution. I was looking through issues to grab an easy first one, and found this. Looks like someone else was going to have a go at it, but never did. I simply added a small change to the FK regex in for Postgres that allows anything not quotes alongside escaped double quotes. Test is included for the scenario mentioned in the issue. Alongside that, I didn't see a test for general quoted strings, so I added another one that includes spaces and dashes, in my experience common things to be used inside quoted identifiers. A manual test as well: DB setup: ``` austin_test_bug=# CREATE TABLE """test_parent_table-quoted""" (id SERIAL PRIMARY KEY, val INTEGER); CREATE TABLE austin_test_bug=# CREATE TABLE test_child_table_ref_quoted (id SERIAL, parent INTEGER, CONSTRAINT fk_parent FOREIGN KEY (parent) REFERENCES """test_parent_table-quoted"""(id)); CREATE TABLE austin_test_bug=# \d+ List of relations Schema | Name | Type | Owner | Persistence | Access method | Size | Description --------+------------------------------------+----------+----------+-------------+---------------+------------+------------- public | "test_parent_table-quoted" | table | postgres | permanent | heap | 0 bytes | public | "test_parent_table-quoted"_id_seq | sequence | postgres | permanent | | 8192 bytes | public | test_child_table_ref_quoted | table | postgres | permanent | heap | 0 bytes | public | test_child_table_ref_quoted_id_seq | sequence | postgres | permanent | | 8192 bytes | (4 rows) ``` And the python: ``` >>> from sqlalchemy import create_engine, inspect >>> engine = create_engine('postgresql://scott:tiger@localhost:5432/austin_test_bug') >>> connection = engine.connect() >>> inspect(connection).get_multi_foreign_keys() {(None, '"test_parent_table-quoted"'): [], (None, 'test_child_table_ref_quoted'): [{'name': 'fk_parent', 'constrained_columns': ['parent'], 'referred_schema': None, 'referred_table': '"test_parent_table-quoted"', 'referred_columns': ['id'], 'options': {}, 'comment': None}]} ``` ### Checklist <!-- go over following points. check them with an `x` if they do apply, (they turn into clickable checkboxes once the PR is submitted, so no need to do everything at once) --> This pull request is: - [ ] A documentation / typographical / small typing error fix - Good to go, no issue or tests are needed - [x] A short code fix - please include the issue number, and create an issue if none exists, which must include a complete example of the issue. one line code fixes without an issue and demonstration will not be accepted. - Please include: `Fixes: #<issue number>` in the commit message - please include tests. one line code fixes without tests will not be accepted. - [ ] A new feature implementation - please include the issue number, and create an issue if none exists, which must include a complete example of how the feature would look. - Please include: `Fixes: #<issue number>` in the commit message - please include tests. **Have a nice day!** Fixes: #10902 Closes: #13179 Pull-request: #13179 Pull-request-sha: 8890dc3 Change-Id: I185c2cb062740551ab931368de602054eb5a4acd (cherry picked from commit c7412ac)
Description
Issue: #10902
Hello! This is my first PR here, so please let me know what I may have missed in terms of having a valuable contribution. I was looking through issues to grab an easy first one, and found this. Looks like someone else was going to have a go at it, but never did.
I simply added a small change to the FK regex in for Postgres that allows anything not quotes alongside escaped double quotes. Test is included for the scenario mentioned in the issue. Alongside that, I didn't see a test for general quoted strings, so I added another one that includes spaces and dashes, in my experience common things to be used inside quoted identifiers.
A manual test as well:
DB setup:
And the python:
Checklist
This pull request is:
must include a complete example of the issue. one line code fixes without an
issue and demonstration will not be accepted.
Fixes: #<issue number>in the commit messageinclude a complete example of how the feature would look.
Fixes: #<issue number>in the commit messageHave a nice day!