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

Psycopg placeholders #198

Closed
snorkysnark opened this issue Jul 13, 2022 · 3 comments · Fixed by #233
Closed

Psycopg placeholders #198

snorkysnark opened this issue Jul 13, 2022 · 3 comments · Fixed by #233
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@snorkysnark
Copy link

Describe the bug
sqlfmt breaks placeholders used by psycopg (python postgres driver), such as %s and %(name)s

To Reproduce
Format the following code:

INSERT INTO some_table VALUES (%s)
INSERT INTO some_table VALUES (%(foo)s)

Expected behavior
The placeholders should remain intact:

insert into some_table values(%s)
insert into some_table values(%(foo)s)

Actual behavior
sqlfmt inserts extra spaces:

insert into some_table values(% s)
insert into some_table values(% (foo) s)

Additional context
What is the output of sqlfmt --version?
sqlfmt, version 0.9.0

@tconbeer
Copy link
Owner

Thanks for the report, @snorkysnark! As it says in the README, we only support select statements for the time being. We'll get to other DDL eventually, but the parsing complexity is much higher (and there is a lot more divergence in different SQL dialects).

Can you use these placeholders in a select statement? We already support other types of placeholders that are parsed by the database, like $1. If you can point me to the specific docs for psycopg2 that discuss this syntax, I can make a better determination for how to prioritize this.

@snorkysnark
Copy link
Author

Yes, they behave similarly in select statements

Here's the relevant documentstion for psycopg2 and psycopg3:
https://www.psycopg.org/docs/usage.html#passing-parameters-to-sql-queries
https://www.psycopg.org/psycopg3/docs/basic/params.html

@tconbeer tconbeer added the enhancement New feature or request label Jul 13, 2022
@tconbeer
Copy link
Owner

tconbeer commented Jul 13, 2022

Thanks for the docs! I love how explicit they are on the format for these placeholders; should make lexing them quite straightforward.

Any interest in contributing the fix for this? Shouldn't be too complicated. You can use this PR as a guide (it added support for $1 and @my_stage as other identifiers): #126

The steps would be:

  1. create a new Rule (in dialect.py) with a regex for matching these placeholders and creating a NAME token
  2. make sure the new Rule's priority is lower than the operator rule
  3. modify the operator rule to match %% (the escaped mod operator in a psycopg string)
  4. add tests in test_dialect.py and add a new file to test_general_formatting.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants