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

Implement Named Parameters #534

Open
itsjamie opened this issue Nov 7, 2016 · 17 comments
Open

Implement Named Parameters #534

itsjamie opened this issue Nov 7, 2016 · 17 comments

Comments

@itsjamie
Copy link

itsjamie commented Nov 7, 2016

The addition of named parameters support in Go 1.8 is documented in #531, tracking this individual change here.

Notes from the document:

Drivers should be clear in their documentation what form of names they accept. 
For example, does the driver support "@ID", "?ID", ":ID", "$ID", "ID". If no 
placeholder is specified and the query language requires one, will the driver 
prepend the default symbol to the name if none are found? This behavior 
should be documented in the driver.

If mixing named parameters with ordinal position, the ordinal position is always 
assigned to, values don't skip named parameter positions. 
For example: "... where ID = :ID and Name = ?", the second placeholder ("?") 
will be ordinal position 2.

I would like to implement this in lib/pq, but figured some discussion would be good. For example, should lib/pq require a specific prefix? I would imagine no because they will be reduced down to $1 and $2 ordinal operators.

I would still want to implement named parameters though so that any shared names in a query can be automatically reduced to the same ordinal parameter.

Thoughts?

@nathany
Copy link

nathany commented Feb 4, 2017

Looks like PostgreSQL doesn't have any special support for named parameters in prepared statements, so is the idea that this would be implemented entirely in the driver?

sql.Named("Name", value) substitutes :Name for $1 in SQL, which receives the value.

Not 100% sure if it's worthwhile at the driver level, since it can also be done at a higher level: http://jmoiron.github.io/sqlx/#namedParams

(But if so, that implementation could be worth looking at)

PostgreSQL does have a named parameter notation when calling functions:
https://www.postgresql.org/docs/current/static/sql-syntax-calling-funcs.html
a => 'Hello' this syntax appears to be 9.5+
a := 'Hello' this syntax is still supported

Not sure if it provides any benefit over passing $1 over to PostgreSQL if the SQL sent to the driver looks the same either way.

@cbandy
Copy link
Contributor

cbandy commented Feb 4, 2017

should lib/pq require a specific prefix? I would imagine no because they will be reduced down to $1 and $2 ordinal operators.

I don't understand what you're asking here.

I would still want to implement named parameters though so that any shared names in a query can be automatically reduced to the same ordinal parameter.

The parameter symbols/markers $n can be used more than once in a query to refer to the same parameter value/argument. Is that what you mean by "shared?" Perhaps I'm misunderstanding what you desire here.

For example, the following query multiplies a number by three:

err := db.QueryRow(`SELECT $1 + $1 + $1`, x).Scan(&y)

@itsjamie
Copy link
Author

itsjamie commented Feb 4, 2017

When speaking to the prefix, I was referencing the google document for database/sql 1.8 changes.

Specifically, there was a section (appears to me gone/edited) that I quoted from the above document in the linked ticket which appeared to suggest the driver pick a prefix and only accept that.

I was initially thinking about just supporting the database/sql changes to ensure it made Go 1.8. Now, particularly it's just a case of options. Also, with named parameters, the driver can do a little grunt work and deduplicate the parameters to the minimal number of ordinal arguments. reducing the query size it might be worthwhile?

Example:

err := db.QueryRow(`SELECT :Num + :Num + :Num`, sql.Named("Num", x)).Scan(&y)

The above should result in the same query in your example, just the driver would dedupe the references and mark them all as the same ordinal argument.

Thanks for your time.

@nathany
Copy link

nathany commented Feb 5, 2017

@itsjamie That deduping makes sense to me, and how I would expect it to work. I'm just an onlooker though, not the project lead.

@dewey4iv
Copy link

I'd really like to see this supported in the driver. @itsjamie has there been any work/plans to see this worked on?

Re: the de-duping - I agree with @nathany - the logic seems sound to me.

@itsjamie
Copy link
Author

itsjamie commented Mar 1, 2017

I haven't planned anything around it, was waiting for someone to give the OK on implementation before starting any work.

@cbandy
Copy link
Contributor

cbandy commented Mar 1, 2017

It seems to me that the only safe way to implement this is with a parser that can handle all forms of quoting. Without it, any prefix we pick will break an existing app out there.

We could skimp on the parser by making the user specify a prefix. In that case, using a literal containing their prefix is their fault. No configured prefix could mean no named arguments. At the same time, configuring lib/pq while staying compatible with libpq connection strings has its challenges.

Re: de-duping. I see why I was confused. In your example the driver receives only one NamedValue. From my perspective there's nothing to reduce/consolidate. Are there any guidelines for how a driver should behave if two NamedValue arrive with the same Name? Perhaps database/sql prevents it? What about a Name that isn't present in the SQL?

@GeertJohan
Copy link

Is this really something that the driver should do? I think sqlx handles this quite well. To me it doesn't feel like this should be handled at the driver level as it's not a postgres feature..
http://jmoiron.github.io/sqlx/#namedParams

@dewey4iv
Copy link

dewey4iv commented Mar 2, 2017

@GeertJohan - Not everyone uses the sqlx package and I don't think they should be required to in order to have access to Go stdlib features. I agree, it isn't a Postgres feature, but it is a feature of database/sql as of Go 1.8.

A quick excerpt from the 1.8 release notes:

"The other features require driver support in database/sql/driver. Driver authors should review the new interfaces. Users of existing driver should review the driver documentation to see what it supports and any system specific documentation on each feature."
https://golang.org/doc/go1.8

To me, at least, this clearly states that it is the responsibility of the driver to implement these features.

@cbandy
Copy link
Contributor

cbandy commented Mar 2, 2017

To me, at least, this clearly states that it is the responsibility of the driver to implement these features.

I agree that a database/sql/driver is the only thing that can implement these database/sql features. My interpretation of the 1.8 release notes is:

  1. drivers may (or may not) implement any number of these features.
  2. drivers should document which features are (and are not) supported.
  3. users should read driver documentation.

@itsjamie
Copy link
Author

itsjamie commented Mar 2, 2017

I agree with your interpretation @cbandy. I submitted this issue because I feel it is likely the next generation of query builders for database/sql will likely use some combination of named parameter values and the typed column data to automate.

Also, when writing composable queries, such as using WITH ( subquery ) as temp UPDATE... it is error-prone to have to rewrite the subquery simply to rename the ordinal parameters, or remember to update the parameter number in the UPDATE query should the subquery change.

@dewey4iv
Copy link

dewey4iv commented Mar 2, 2017

@cbandy - I agree. My interpretation was assuming that you agreed to adding this feature. @itsjamie - that would be a nice feature to have!

@buyology
Copy link

buyology commented Jun 7, 2017

any progress on this? 😊 🚀

@jeromedoucet
Copy link

jeromedoucet commented Mar 28, 2019

I will try to work on it. Working on a Golang backend with huge SQL query, such feature can help us a lot to have cleaner SQL code.

@flibustenet
Copy link

@jeromedoucet there is an issue in sqlx about named query that can help you
jmoiron/sqlx#206
with @detaoin proposing a sqltokenizer https://github.com/detaoin/sql2http/blob/master/sqltokenizer.go

I'm not sure it should be implemented in a driver, but if it will be i will use it !

@detaoin
Copy link

detaoin commented Mar 28, 2019

Hi @jeromedoucet
The simple sqltokenizer @flibustenet mentions was implemented because I needed named parameters for various driver backends. And sqlx doesn't handle quoting and comments (it's a simple substring matching approach).
However the code I wrote hasn't been deeply tested. (I think) I'm the only user for now. If you intend to use it, that be a great motivation for me to start writing some serious testing.

Let me know if I can help.

jeromedoucet added a commit to jeromedoucet/pq that referenced this issue Apr 11, 2019
what:
Add named parameters support for "conn". It works basically like ordinals
parameters, but with an alphabetical label prefixed with ":"

Example:

Select $1, $2, $1; may become Select :firstName, :lastName, :firstName

Why:
SQL queries may be difficult to maintain, especially when the complexity
is growing. Ordinal parameters are great, but may not be meaningfull for
the one who is trying to understand the query.

The goal here is not to have something full featured and complex : pq is
"just" a driver, but having a thin layer, simple to maintain, that will
fit 90% of the needs.

Limitations:
Its simplicity means that there is many limitations :

First of all, using mixed types of parameters is not
allowed. Man can not use ordinal parameters and named
parameters is the same query.

Refs lib#534
jeromedoucet added a commit to jeromedoucet/pq that referenced this issue Apr 12, 2019
Add explicit cast for some postgres version.

Refs lib#534
@bmizerany
Copy link
Contributor

bmizerany commented Apr 23, 2021

I too want this, but I also understand the concerns and hesitations around bringing it in. I have a workaround for now, and few ideas and would love feedback on:

Currently I use this workaround:

func prep(q string, args ...interface{}) (qq string, newArgs []interface{}) {
        // TODO(bmizerany): submit PR for lib/pq to support named params

        var keys []string // as strings.NewReplacer requires
        var vals []interface{}
        
        for i, a := range args {
                if na, ok := a.(sql.NamedArg); ok {
                        keys = append(keys, fmt.Sprintf("@%s", na.Name))
                        keys = append(keys, fmt.Sprintf("$%d", i+1))
                        vals = append(vals, na.Value)
                } else {
                        vals = append(vals, a) 
                }
        }
        q = strings.NewReplacer(keys...).Replace(q)
        return q, vals
} 

and use it like:

        qq, args := prep(q,
                sql.Named("before", before),
                sql.Named("topic", topic),
                sql.Named("n", n),
        )
        
        rows, err := m.db.QueryContext(ctx, qq, args...)
        if err != nil {
                return nil, err
        }

It's a bit crude and naive, but it works for basic use cases.

To land support in lib/pq I propose an incremental approach:

  1. Introduce lib/pqx: This will be an experimental driver that is a thin shell over lib/pq. The idea being it coerces queries into what lib/pq supports today while supporting sql.NamedArg.
  2. In lib/pqx support a new connection string option named=@. If present, Query, Exec, et al will play nicely with NamedArg. This allows for backwards compatibility and non-interference with other experimental features in pqx that may one day be introduced.
  3. If the community is accepting and we decided to move forward, we can port sql.NamedArg support into lib/pq after enough vetting and hardening in pqx.

I'm excited to hear any feedback!

-b

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

10 participants