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

Feat(parser): improved comment parsing #1956

Merged
merged 3 commits into from
Jul 25, 2023

Conversation

mpf82
Copy link
Contributor

@mpf82 mpf82 commented Jul 25, 2023

improved comment parsing for INSERT, UPDATE, and DELETE

added tests with and w/o CTEs

improved comment parsing for INSERT, UPDATE, and DELETE

added tests with and w/o CTEs
@mpf82
Copy link
Contributor Author

mpf82 commented Jul 25, 2023

The build process is complaining about this line:

comments = comments if comments else None

sqlglot/parser.py:1718: error: Incompatible types in assignment (expression has type "list[Any] | None", variable has type "list[Any]") [assignment]

I am trying to make sure comments either has elements or - if the list is empty - is set to None.

I do not know how to make mypy happy, help is appreciated.

Copy link
Collaborator

@georgesittas georgesittas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Maybe we should also test the SQL generation for these comments?

I am trying to make sure comments either has elements or - if the list is empty - is set to None. I do not know how to make mypy happy, help is appreciated.

You can also just inline it in the self.expression call:

        return self.expression(
            exp.Insert,
            comments=comments,
            ...

You don't need to set comments to None if it's an empty list, the expression method doesn't care if it's None or [].

tests/test_parser.py Show resolved Hide resolved
tests/test_parser.py Show resolved Hide resolved
@georgesittas georgesittas changed the title improved comment parsing Feat(parser): improved comment parsing Jul 25, 2023
@mpf82
Copy link
Contributor Author

mpf82 commented Jul 25, 2023

Maybe we should also test the SQL generation for these comments?

Sadly, in all cases, the comments are moved to the end, instead of at the beginning.

I am not even sure, how select_sql achieves comments in front in the generator.

make mypy happy
@georgesittas
Copy link
Collaborator

Maybe we should also test the SQL generation for these comments?

Sadly, in all cases, the comments are moved to the end, instead of at the beginning.

I am not even sure, how select_sql achieves comments in front in the generator.

There's a flag in the generator that controls this, it's called WITH_SEPARATED_COMMENTS. You should be able to fix this by updating it to contain exp.Insert etc.

Added indentity examples:

/* comment1 */ INSERT INTO x /* comment2 */ VALUES (1, 2, 3)
/* comment1 */ UPDATE tbl /* comment2 */ SET x = 2 WHERE x < 2
/* comment1 */ DELETE FROM x /* comment2 */ WHERE y > 1
@tobymao tobymao merged commit 59847f5 into tobymao:main Jul 25, 2023
@mpf82 mpf82 deleted the improved-comment-parsing branch July 27, 2023 13:31
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.

3 participants