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: sql argument counts are off by one when reporting an error #15676

Closed
lpar opened this issue May 13, 2016 · 9 comments
Closed

database/sql: sql argument counts are off by one when reporting an error #15676

lpar opened this issue May 13, 2016 · 9 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@lpar
Copy link

lpar commented May 13, 2016

I know this is pretty trivial, but when reporting an error the sql package seems to report arguments by counting them starting at zero.

Since SQL arguments are generally numbered from 1 -- and indeed, for PostgreSQL they are numbered $1, $2, $3... in the SQL itself -- this confused me.

e.g. sql: converting Exec argument #3's type: unsupported type main.Date, a struct
It was actually argument #4 ($4 in the SQL statement).

go version go1.6.2 linux/amd64

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/meta/go/tecdocs"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GO15VENDOREXPERIMENT="1"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

@ianlancetaylor ianlancetaylor changed the title sql argument counts are off by one when reporting an error database/sql: sql argument counts are off by one when reporting an error May 13, 2016
@quentinmit
Copy link
Contributor

I suspect any changes we make at this point are just going to make things even more confusing, unfortunately. Would dropping the "#" prefix make you less likely to think they're 1-based?

/cc @bradfitz ?

@quentinmit quentinmit added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 17, 2016
@quentinmit quentinmit added this to the Go1.8 milestone Jun 17, 2016
@kardianos
Copy link
Contributor

I'm of the opinion we should ensure all parameters are referenced by ordinal, not index.

@bradfitz
Copy link
Contributor

bradfitz commented Oct 9, 2016

@kardianos, you mean you advocate for a change, rather than documenting the status quo?

@kardianos
Copy link
Contributor

Yes, I would change the message to ensure all parameters match the ordinal position. Simply being SQL is reason enough to think they are 1-based references (ordinal not index).

@bradfitz
Copy link
Contributor

Sure. We can change it if everybody agrees. Should we preface them all by '$' then? Is that more clear?

@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. Documentation and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 17, 2016
@gopherbot
Copy link

CL https://golang.org/cl/31262 mentions this issue.

@lpar
Copy link
Author

lpar commented Oct 17, 2016

How about keeping # as the prefix for 0-indexed, but deprecating it; and introducing $ as the prefix for 1-indexed? That'd presumably not break anyone's code unexpectedly, and then everyone could have until Go 1.9 to switch over.

@kardianos
Copy link
Contributor

@lpar What do you think of the CL https://golang.org/cl/31262 ? I hope no one is trying to parse the error message mechanically, but if they are, I can include it in a general announcement in a week or two.

@lpar
Copy link
Author

lpar commented Oct 19, 2016

I think the improved message would at least have eliminated my confusion after the fact.

@golang golang locked and limited conversation to collaborators Oct 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants