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

Preserve comments #15

Closed
mdeland opened this issue Jun 22, 2016 · 7 comments
Closed

Preserve comments #15

mdeland opened this issue Jun 22, 2016 · 7 comments

Comments

@mdeland
Copy link

mdeland commented Jun 22, 2016

Is there anyway way to preserve comments in the output of the queryparser? It seems that comments aren't part of the pg parsetree, but it would be nice to be able to preserve their content/location somehow.

@lfittl
Copy link
Member

lfittl commented Jun 22, 2016

@mdeland Thats a good question - I think it would unfortunately be quite invasive to patch up the parser.

It seems PostgreSQL treats comments as whitespace in the lexical scanner: https://github.com/postgres/postgres/blob/master/src/backend/parser/scan.l#L201

Since pg_query just takes that same code, it has to live within those constraints. Whats the use case for you?

I assume we might actually have an easier time using a regular expression to find the comments (and we could look into making that easier, if helpful).

@mdeland
Copy link
Author

mdeland commented Jun 22, 2016

Yeah, I assumed the scanner was probably just ignoring the comments since no internal PG node representation represents a comment. I am trying to use the output here to format sql, but it would be nice to preserve the comments interspersed where they fall within the query.

@lfittl
Copy link
Member

lfittl commented Jun 22, 2016

@mdeland Yeah, that makes sense.

I don't think there's an easy way to support this as it stands - I'd be willing to integrate a small parser patch, since I'm already carrying one to support parsing ? replacement characters (https://github.com/lfittl/libpg_query/blob/9.5-latest/patches/01_parse_replacement_char.patch).

Do you think you'd be interested in diving into the C code yourself and see if you can make this work? (I'm too short on time right now to do so)

@mdeland
Copy link
Author

mdeland commented Jun 22, 2016

I'll take a look at it - I'm not familiar with the bison (?) parser framework but could be interesting.

@lfittl
Copy link
Member

lfittl commented Jun 22, 2016

@mdeland Yeah, I'll warn you though - it can be a bit painful :-)

Note also that we want to avoid making any invasive changes (i.e. changing the way things work too much), since this needs to be synced with every new PostgreSQL release.

In any case, have fun!

@lfittl
Copy link
Member

lfittl commented Aug 30, 2017

Just adding a note here that pg_hint_plan also needs to parse comments, so we may be able to borrow some code: https://github.com/ossc-db/pg_hint_plan/blob/c7888a113ac952c39af91c2d448f864d06e73837/pg_hint_plan.c#L1899

Also note that pg_hint_plan is licensed under BSD-3-clause, so if we do end up using some of their code we should attribute accordingly.

@lfittl
Copy link
Member

lfittl commented Mar 30, 2021

Closing this, since this has been implemented in the scanner method released in pg_query 2.0:

https://github.com/pganalyze/libpg_query#usage-scanning-a-query-into-its-tokens-using-the-postgresql-scannerlexer

(note it doesn't really fit the logic to put this information in the parser, so you would have to use both the scanner and the parser method in some cases)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants