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

database/sql: ? vs $ inconsistency #3602

Closed
rsc opened this issue May 8, 2012 · 32 comments
Closed

database/sql: ? vs $ inconsistency #3602

rsc opened this issue May 8, 2012 · 32 comments
Assignees
Milestone

Comments

@rsc
Copy link
Contributor

@rsc rsc commented May 8, 2012

do we care that you have to use different wild cards depending on the back end?




---------- Forwarded message ----------
From: zeroc8 <arnold.angerer@zeroc8.com>
Date: Tue, May 8, 2012 at 2:03 PM
Subject: [go-nuts] Re: Postgres pq - LastInsertId
To: golang-nuts@googlegroups.com

Am Dienstag, 8. Mai 2012 11:19:13 UTC+2 schrieb Alexander Neumann:
>
> Try something like this (untested):
>
> err := db.QueryRow("INSERT INTO foo (bar) VALUES ($1) RETURNING id;",
> bar).Scan(&id)
>
>
Thanks, works.
err := tx.QueryRow("insert into car(name) values($1) returning
id","test").Scan(&id)
@rsc
Copy link
Contributor Author

@rsc rsc commented Sep 12, 2012

Comment 1:

Labels changed: added go1.1maybe.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 8, 2012

Comment 2:

I think different back ends will naturally use different symbols here. I see no reason
you couldn't pass a args struct that has both a value and a name, so you can have a
named arg either. I would say the driver and back end should do what is natural.
SQL is different enough as it is. IE, you are not going to execute pg/SQL on sqlite.
@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 8, 2012

Comment 3:

Also, it not just "$" vs "?", Some use @varName and in the sql you might use
declare @myvar int
select @myvar = t.Foo
from MyTable t
where ID = @myid
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 29, 2012

Comment 4 by raul.san@sent.com:

Related: https://groups.google.com/forum/?fromgroups=#!topic/golang-nuts/p-QPMneatDI
@rsc
Copy link
Contributor Author

@rsc rsc commented Dec 10, 2012

Comment 5:

Labels changed: added size-m.

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 27, 2013

Comment 6 by raul.san@sent.com:

You must have in mind that there are two kind of strings which can be different in each
SQL engine: the place holders and also the quotes.
So, my solution has been to use a "template" with "{P}" instead of place holder string,
and "{Q}" instead of the quote string. Then, they're replaced according to the SQL
engine.
Here it's the function that I use to replace:
https://github.com/kless/modsql/blob/7c86ebaca0a8bef8bba2afa2c6ec4c0f9e5e1b82/modsql.go#L75
and like example of statements, where I'm using:
https://github.com/kless/modsql/blob/7c86ebaca0a8bef8bba2afa2c6ec4c0f9e5e1b82/testdata/model.go#L50
* * *
The function that I use to replace could be taken like idea to implement something
similar in the standard library.
@rsc
Copy link
Contributor Author

@rsc rsc commented Mar 12, 2013

Comment 7:

[The time for maybe has passed.]

Labels changed: removed go1.1maybe.

@rsc
Copy link
Contributor Author

@rsc rsc commented Jul 30, 2013

Comment 8:

Brad, could you decide whether this should be fixed or closed and comment on the bug?

Labels changed: added go1.2maybe.

@rsc
Copy link
Contributor Author

@rsc rsc commented Jul 30, 2013

Comment 9:

Labels changed: added feature.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Aug 2, 2013

Comment 10:

I'm in no rush to change anything here, since I'm not sure what a safe solution is.
I really don't want to get into the business of parsing SQL, or worse: a dozen different
SQL dialects.
I'm also reluctant to encourage driver authors to parse their SQL variant and and morph
it into a Go variant (e.g. "use question marks for placeholders!").
Doing some research I found
http://sqlrelay.sourceforge.net/sqlrelay/programming/binds.html which says:
"""
Different databases have different syntax for bind variables. Oracle bind variables are
names preceeded by a colon. In MySQL, DB2 and Firebird, bind variables are represented
by question marks. In Sybase and MS SQL Server, bind variables are names preceeded by an
@ sign. In PostgreSQL, bind variables are numbers preceeded by a $ sign.
"""
What do other database libraries do?
We could have a mini SQL lexer and say that any ? not in a quoted string can be replaced
by the driver (optional hook) with the appropriate driver-specific positional bind
variable.
@rsc
Copy link
Contributor Author

@rsc rsc commented Aug 2, 2013

Comment 11:

The ? replacer sounds fine to me. It could just be a function in database/driver that
drivers can call if they want it.
@ngrilly
Copy link

@ngrilly ngrilly commented Aug 15, 2013

Comment 12:

The ? replacer is very inconvenient when an SQL query uses the same variable in many
places.
In such a case (which is quite frequent), the alternative solutions are better ($1 or
:varname or @varname).
@robpike
Copy link
Contributor

@robpike robpike commented Aug 16, 2013

Comment 13:

Uncertain. Deferring to Go1.3.

Labels changed: added go1.3maybe, removed go1.2maybe.

@robpike
Copy link
Contributor

@robpike robpike commented Aug 20, 2013

Comment 14:

Labels changed: removed go1.3maybe.

@rsc
Copy link
Contributor Author

@rsc rsc commented Nov 27, 2013

Comment 15:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor Author

@rsc rsc commented Nov 27, 2013

Comment 16:

Labels changed: removed feature.

@rsc
Copy link
Contributor Author

@rsc rsc commented Dec 4, 2013

Comment 17:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor Author

@rsc rsc commented Dec 4, 2013

Comment 18:

Labels changed: added repo-main.

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 8, 2013

Comment 19 by raul.san@sent.com:

I use the next function to replace the global character for the place holder by the used
one into a specific SQL engine:
const _PLACEHOLDER = "?"
// SQLReplacer replaces the characters "$" with the placeholder parameter for
// the given SQL engine.
func SQLReplacer(eng Engine, src string) string {
    switch eng {
    case MySQL, SQLite:
        if _PLACEHOLDER != "?" && strings.Contains(src, _PLACEHOLDER) {
            return strings.Replace(src, _PLACEHOLDER, "?", -1)
        }
    case Postgres:
        for nParam := 1; strings.Contains(src, _PLACEHOLDER); nParam++ {
            src = strings.Replace(src, _PLACEHOLDER, fmt.Sprintf("$%d", nParam), 1)
        }
    default:
        panic("engine not supported: " + eng.String())
    }
    return src
}
@kardianos
Copy link
Contributor

@kardianos kardianos commented Aug 19, 2014

Comment 20:

I recommend this be closed as "can't fix".
In order to do such a replacement, you would need to implement a micro SQL parser that
understands comments, strings, and identifiers. Comments vary from system to system (--,
/* */ ...), strings are fairly normal (''), but identifiers ("", []) vary as well.
I'm writing several drivers where I have reason to parse the SQL at this level, but it
is at the driver level and it is to work in multiple commands or command groups, and
substituting names in the SQL to ordinal position (for postgresql, this is for the rdb
front-end which allows named parameters). But again, this is very unique to the driver
code. If a driver wants to normalize it, then let the driver do it's thing.
Also, "re-using" sql shouldn't be an issue here. Anything above the most trivial
statements will not port without changes between systems. And a common sql dialect that
translates into system specific sql is well beyond this package's scope.
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Aug 19, 2014

Comment 21:

I was thinking more along the lines of letting drivers do their own lexing and
replacement. Some optional hook they can implement. I haven't thought about it much,
like which syntax we pick to be the common one that gets mapped to others. It's related
to having named placeholders too.
@rsc rsc added accepted labels Aug 19, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed accepted labels Apr 14, 2015
@nathany
Copy link
Contributor

@nathany nathany commented Jun 19, 2015

As @kardianos points out, even if placeholders were made consistent, there are other differences between the SQL syntax from one DB driver to the next.

Personally, I think it's better to leave this to the domain of SQL builders, and not start down the slippery slope of trying to have a common dialect, even if only one small aspect of it. My 2 cents.

@bradrydzewski
Copy link

@bradrydzewski bradrydzewski commented Sep 15, 2015

I agree that this is probably out of scope for the driver. I do, however, think we could make this a bit easier to for developers to manage by exposing the driver name:

type Driver interface {
        Open(name string) (Conn, error)
+       Name() string
}

This would allow me to use the existing Driver() function to determine which driver has been loaded and adjust the SQL query as needed:

switch db.Driver().Name() {
case "postgres":
  // replace ? with $%d
case "mysql", "sqlite3":
}

This would probably require exposing either the DB or Driver in the transaction as well:

type Tx struct {
    db *DB
}

+func (t *Tx) Driver() driver.Driver {
+    return t.db.Driver()
+}

I think this sort of minor change would make this problem a bit easier to deal with. I've noticed projects writing custom functions to normalize queries (see sqlx.Rebind as an example) most of which end up wrapping sql.DB just to store and provide access to the the driver name.

@kostya-sh
Copy link
Contributor

@kostya-sh kostya-sh commented Sep 16, 2015

The first change you propose is backward incompatible and thus cannot be done. If Name() method is added to the Driver interface then all existing drivers will stop implementing Driver interface.

@minux
Copy link
Member

@minux minux commented Sep 16, 2015

@bradrydzewski
Copy link

@bradrydzewski bradrydzewski commented Sep 16, 2015

We could adjust the approach without breaking the API guarantee. We could store the driverName in the DB structure:

type DB struct {
    driver driver.Driver
+   name   string
    dsn    string

Which is available at the time of opening a connection:

func Open(driverName, dataSourceName string) (*DB, error) {
    ...
    driveri, ok := drivers[driverName]
    ...
    db := &DB{
        driver:   driveri,
+       name:   driverName,
        dsn:      dataSourceName,
        openerCh: make(chan struct{}, connectionRequestQueueSize),
        lastPut:  make(map[*driverConn]string),
    }
    ...
}

And could be exposed by a function on the DB struct:

+func (db *DB) Name() {
+   return db.name
+}
@mattn
Copy link
Member

@mattn mattn commented Sep 16, 2015

Note that driver name is possible to be user-defined.

https://github.com/mattn/go-sqlite3/blob/master/_example/custom_func/main.go#L65

@kardianos
Copy link
Contributor

@kardianos kardianos commented Nov 28, 2016

I don't see how this can be realistically done across different database vendors. For simple queries, this might be possible, but not for full T-SQL or PL/SQL or pgPL/SQL queries, esp when the query definition starts going into DDL.

If you want to expose the driver name, open a separate issue for that. The various use cases can be discussed there.

@kardianos kardianos closed this Nov 28, 2016
@timbunce
Copy link

@timbunce timbunce commented May 29, 2017

I just ran across this issue as I'm stumbling my way into Go and database access.

Historical footnote:

The Perl DBI gained a lot by mandating over 20 years ago that drivers be required to support ? for placeholders. (They can support their natural placeholders as well.) The query composition of SQL::Abstract stands on the shoulders of that feature. And DBIx::Class builds on SQL::Abstract.

@kardianos
Copy link
Contributor

@kardianos kardianos commented May 29, 2017

@timbunce Closed issues aren't tracked.

From your comment I'm unsure what DBI gained by requiring support for ? placeholders, nor does it address the issues presented above, such as various query dialects, nor does it address systems that don't use positional placeholders at all, but only uses named placeholders.

@timbunce
Copy link

@timbunce timbunce commented May 29, 2017

Hello @kardianos. I appreciate that closed issues aren't tracked. It was just a historical footnote, albeit one that wasn't very clear. To your points:

  • ? placeholders allow a fragment of SQL, coupled with a corresponding list of values for any placeholders, to be passed around and later composed into larger SQL statements without needing to worry about the numbering of $1 style placeholders, or generating and tracking arbitrary names for :foo style placeholders.
  • Parsing sufficient to handle placeholders isn't as hard as it may seem, especially as it would be done by the drivers that know their own SQL dialect.
  • ? style placeholders can easily be converted into either of the other styles.

Subsequent to my original comment above I've discovered that sqlx supports ? placeholders, although it barely gets a mention in the guide.

@samuelkaufman
Copy link

@samuelkaufman samuelkaufman commented Oct 25, 2017

@timbunce if you're at all interested I've been generating SQL with DBIx::Class and using it in a go project with a reasonable amount of success. I'm now generating SQLite queries as well as the Postgresql queries I started with it has been holding up.

@golang golang locked and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.