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

Pretty printing: Quoting in table names #3

Closed
portnov opened this issue Oct 18, 2011 · 3 comments
Closed

Pretty printing: Quoting in table names #3

portnov opened this issue Oct 18, 2011 · 3 comments

Comments

@portnov
Copy link

portnov commented Oct 18, 2011

PostgreSQL (and, I think, many other RDBMs) requires table identifiers to be quoted in many cases. But HsSqlPpp does not quote them.

For example, printStatement generates:

select ... from Record r, Submmission s ...

while we need:

select ... from "Record" r, "Submission" s ...

Seems such a quoting might be done with simple patch. In Pretty.lhs replace convDqi definition with following:

convDqi :: SQIdentifier -> Doc
convDqi (SQIdentifier _ is) = hcat $ punctuate (text ".") $ map (text . quot) is
where quot x = """ ++ x ++ """

@JakeWheat
Copy link
Owner

Thanks for the report!

No thought has been given to the issue of quoted identifiers, even though the parser sort of handles them, so thanks for the patch. I will try to see if there is anything else preventing the proper handling of identifiers that need quoting.

Let me know if you have any other issues.

@JakeWheat
Copy link
Owner

I've looked into this a little more:
the abstract syntax doesn't record whether an identifier appears quoted. It preserves the case of the identifier (I think). The type check ignores case (which is wrong for quoted identifiers). The pretty printer is unable to tell whether to quote an identifier if it e.g. uses letters only.

So parsing something like 'select * from Reader' could be parsed then pretty printed as either 'select * from Reader' or 'select * from "Reader"'. If the table name in the catalog is e.g. 'reader', then the original sql is fine, but outputting the identifier with quotes would change the meaning of the sql query.

I think the correct solution is to represent quoted and unquoted identifiers explicitly in the ast, (with fixes to the parser, typechecker and pretty printer). This doesn't seem too difficult and I have put it on my todo list.

@JakeWheat
Copy link
Owner

fixed in this commit:

f2339a9

not comprehensively tested, please reopen bug with example if you find something still broken for quoted identifiers

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