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

Quote columnrefs #40

Merged
merged 2 commits into from
Dec 3, 2015
Merged

Quote columnrefs #40

merged 2 commits into from
Dec 3, 2015

Conversation

avinoamr
Copy link
Contributor

@avinoamr avinoamr commented Dec 2, 2015

Column refs could contain reserved keywords when parsed, like a column called "user", unlike the user keyword which is parsed into the current_user() function call. The deparse algorithm disregards this distinction by turning the column into the function call incorrectly.

For example, in the following query:

SELECT "user" FROM users

The "users" target list would be correctly parsed into

{
    "COLUMNREF": {
        "fields": [ "users" ]
    }
}

But deparsing this structure results in the following, incorrect query where user is a reserved keyword.:

SELECT user FROM users

This is fixed here by quoting all columnrefs.

@lfittl
Copy link
Member

lfittl commented Dec 2, 2015

Good point - I think you are correct that we should simply quote all column refs to be on the safe side.

Could you update the tests to reflect this as well? (see failing Travis CI build)

Thanks for contributing! :)

@lfittl lfittl added the bug label Dec 2, 2015
@avinoamr
Copy link
Contributor Author

avinoamr commented Dec 3, 2015

@lfittl Done.

@lfittl
Copy link
Member

lfittl commented Dec 3, 2015

@avinoamr Thanks, I know that was quite some more work, appreciate it!

lfittl added a commit that referenced this pull request Dec 3, 2015
@lfittl lfittl merged commit 03d2524 into pganalyze:master Dec 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants