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

Qualified wildcard to_string() is invalid #52

Closed
thomas-jeepe opened this issue Apr 22, 2019 · 4 comments
Closed

Qualified wildcard to_string() is invalid #52

thomas-jeepe opened this issue Apr 22, 2019 · 4 comments

Comments

@thomas-jeepe
Copy link
Contributor

Line in question: https://github.com/andygrove/sqlparser-rs/blob/master/src/sqlast/mod.rs#L114

Calling to_string() on a qualified wildcard returns Qualification* instead of Qualification.*. Should be as simple as changing the line to: q.join(".") + ".*"

@nickolay
Copy link
Collaborator

Thanks! While this fix should probably be applied anyway, I wonder how you found this.

Do you know which dialects allow qualified wildcards "inside" expressions, like count(foo.*) in your PR's testcase? ANSI SQL defines <aggregate function> ::= COUNT <left paren> <asterisk> <right paren> ..., MSSQL agrees by reporting a syntax error in count(foo.*), and the construct doesn't make sense to me. I guess COUNT(distinct foo.*) could make sense, but I don't know of a RDBMs that supports that either.

(The widely supported SELECT foo.* is handled correctly now, because we convert ASTNode::SQLQualifiedWildcard to SQLSelectItem::QualifiedWildcard here -- I was initially confused how this was not caught by an existing test.)

@thomas-jeepe
Copy link
Contributor Author

thomas-jeepe commented Apr 22, 2019

I am not too sure exactly which databases support it, but it is a valid syntax.

Stack overflow link recommending its use.

I would have to manually test it in different databases to see which support it.

@nickolay
Copy link
Collaborator

Thanks! no need to test, I was wondering if you knew one off-hand. Turns out Postgres does support it, though I didn't find it mentioned in their docs.

@nickolay
Copy link
Collaborator

The PR was merged. Thanks again!

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

No branches or pull requests

2 participants