-
Notifications
You must be signed in to change notification settings - Fork 18k
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: NamedParam and Param are wrong names #18099
Comments
/cc @kardianos |
SGTM |
I'll create an example shortly. I've made this change in a local CL, but one thing I noticed that everywhere else they are referred to as "Parameters" from DefaultParameterConverter to comments. There are two places where we reference something called "args", in the comment above the function In MS-SQL it universally references Parameter for SQL function input values: In sqlite, it uses argument for the C API and parameter for the SQL arguments: https://www.sqlite.org/c3ref/bind_blob.html In postgresql documentation, argument and parameter are both used: https://www.postgresql.org/docs/9.4/static/xfunc-sql.html I don't care strongly, but I think because we are already using Parameter in parts of the exported API already we should change |
CL https://golang.org/cl/33660 mentions this issue. |
CL https://golang.org/cl/33663 mentions this issue. |
I mailed both a rename per @rsc and just a comment and argument rename per my suggestion. Take the one you'd like. |
"named parameter" is the only name used for these in every SQL implementation out there... I think calling them "named arguments" will be very confusing for users. |
@rsc, you cool with struct named |
The "?name" syntax in the SQL string is a parameter. The value passed to Query or Exec is an argument. During the execution of the SQL statement, the argument is bound to the parameter. @quentinmit, without a citation to what you are talking about, it's very hard to respond usefully. All I will say is that yes, "named parameter" is a widespread (and accurate) term for the concept of the named placeholders in the SQL queries. But the Go value being passed to Query or Exec that is bound to the named parameter during the SQL execution is a named argument. @kardianos, thanks for the links. The MS-SQL docs are using parameter correctly. They use "value" where I am suggesting using "argument". The sqlite docs are using parameter and argument correctly. The postgresql docs also seem to be using parameter and argument correctly (they're very long and I didn't read the whole page). We should be precise, like all the other implementations are precise. The value passed to Query/Exec is an argument, not a parameter. The struct should be NamedArg, or NamedArgument, certainly not NamedParameter. It could reasonably be NamedValue but that's in driver and I wasn't sure if that would be too confusing. |
CL https://golang.org/cl/33760 mentions this issue. |
Updates #18099 Change-Id: I16b4b2dd881d63cbb406d14a4fd960f0a777a452 Reviewed-on: https://go-review.googlesource.com/33760 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sorry posting comment on closed issue. @kardianos you've better to update this too: https://docs.google.com/document/d/1F778e7ZSNiSmbju3jsEWzShcb8lIO4kDyfKDNm4PNd8/edit# |
If you have
then x is a function parameter and 42 is a function argument.
The new API sql.NamedParam and sql.Param are using the term parameter when they should be using argument. This is very confusing and it even contradicts the names used in the current docs: Exec takes
args ...interface{}
notparams ...interface{}
.I suggest changing the type to sql.NamedArg and the constructor to sql.Named. Also sql.Named needs a good example. It can be short, but something like
would help.
The text was updated successfully, but these errors were encountered: