Skip to content

Support ? as a placeholder value #233

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

Closed
laurent22 opened this issue Feb 13, 2014 · 5 comments
Closed

Support ? as a placeholder value #233

laurent22 opened this issue Feb 13, 2014 · 5 comments
Labels

Comments

@laurent22
Copy link

The current driver doesn't support ? as a placeholder, although Postgres apparently does support it (see here).

All the other database/sql drivers seem to handle ? as well, and not having it supported by the Postgres one makes it cumbersome to write generic SQL-based code because it means all the prepared statements must be written to take into account the current driver. This is made even more complicated by the fact that database/sql doesn't allow querying the current driver.

@johto
Copy link
Contributor

johto commented Feb 13, 2014

The current driver doesn't support ? as a placeholder, although Postgres apparently does support it (see here).

That's EPCG, which is in a way a client itself, just like pq. But they have the luxury of actually having a copy of the SQL parser of postgres.

All the other database/sql drivers seem to handle ? as well, and not having it supported by the Postgres one makes it cumbersome to write generic SQL-based code because it means all the prepared statements must be written to take into account the current driver. This is made even more complicated by the fact that database/sql doesn't allow querying the current driver.

I don't have much sympathy for people trying to write "generic SQL-based code". That said, I don't think I would oppose to a properly implemented, defaults-to-off feature that implemented this. We wouldn't need a full parser, but we would have to know whether we're inside quotes or not. All the other quote types ("", E'', the unicode bunch) should be quite easy AFAIK, but to know whether we're inside single quotes or not, we'd also have to take into account the standard_conforming_strings value of the server.

Even if you got all the quoting right, there's still quite a lot of code to write to support the database/sql interface. So I wouldn't hold my breath.

@laurent22
Copy link
Author

people trying to write "generic SQL-based code"

Hmm, what is wrong with writing generic code? There are use cases for it, and in fact it's pretty much possible today with database/sql (except for the Postgres driver exception).

Thanks for the feedback though. I've noticed that the jmoiron/sqlx package handles this by parsing the query and renaming ? to $x. I wonder if a similar solution could be implemented here? That involves an additional conversion step so it's probably not that efficient though.

@johto
Copy link
Contributor

johto commented Feb 13, 2014

Thanks for the feedback though. I've noticed that the jmoiron/sqlx package handles this by parsing the query and renaming ? to $x. I wonder if a similar solution could be implemented here? That involves an additional conversion step so it's probably not that efficient though.

That's sort of the thing, really. If you're fine with a solution with caveats that big, it's not difficult to implement it in your "generic" layer. Which you will need if you use a database for something more than a K/V store; consider UPSERTing, constraint violations, differences in the planners and executors requiring writing some queries differently, etc.. If you want a solution good enough for everybody, you'll need to do way better than that.

@paulhammond
Copy link
Contributor

If you want an idea of some of the other differences you'll need to implement it's worth taking a look at the Dialect type of jmoiron/modl (written by the author of sqlx) or the Dialect type of coopernurse/gorp. Note that both only implement basic syntactic differences between SQL dialects, and most of the things @johto mentioned aren't covered.

Switching between ? and $1 really is the easy tip of the iceberg when it comes to writing generic database code.

@msakrejda
Copy link
Contributor

I agree with @johto and @paulhammond. Trying to write database-independent code without a dialect layer will only take you so far, and trying to force complexity into the driver to accomplish this does not seem like a good idea. I'd much rather depend on Postgres to deal with parameters than try to implement some sort of bastardized SQL grammar parsing in the driver to support non-native parameter markers.

Note that to do this correctly, we'd have to avoid replacing parameter markers in comments, string literals (including all the dollar-quoting flavors), identifiers, and aliases. E.g., SELECT $x$?$x$ as "?", "?" as foo, ? as bar FROM baz should turn into SELECT $x$?$x$ as "?", "?" as foo, $1 as bar FROM baz.

This may seem like an extremely contrived example, but sooner or later, you'll hit some subset of this in real-world (possibly ORM-generated) queries, and being sloppy here leads at best to weird, hard-to-debug errors, and at worst to SQL injection.

I'm closing this as wontfix. If a patch shows up with serious test coverage for these edge cases, we can revisit, but I honestly don't think this is an avenue worth pursuing in the driver.

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

No branches or pull requests

4 participants