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

Refactored Rebind() so that it respects escaped ?'s #768

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danielmorell
Copy link

@danielmorell danielmorell commented Aug 23, 2021

This PR addresses a FIXME comment I found next to the Rebind() function.

FIXME: this should be able to be tolerant of escaped ?'s in queries without losing much speed, and should be to avoid confusion.

Summary of changes

  • Refactored Rebind() to respect ? that are properly escaped as part of a string using single or double quotes.
  • Moved existing Rebind() unit tests from sqlx_test.go to bind_test.go
  • Refactored existing Rebind() unit tests to run along side the new unit tests.
  • Moved Rebind() benchmarks to bind_test.go and combined them into a single benchmark function with sub-benchmarks.

Valid Escaped ?'s

Because of the cross DBMS nature of Rebind() there are some ways to escape a character or string in Postgres, SQL Server, etc. that don't make sense to implement because they are not supported by all DBMS's.

This version of Rebind() respects two kinds of escaped question marks: string constants, and quoted identifiers. Double an single quotes that are inside a string constant or quoted identifier can be escaped by using a C style backslash e.g. \" or by using a two quotation marks e.g. ""

String Constants

A ? that is part of a valid string constant i.e. surrounded by single quotes is not replaced with a parameter placeholder e.g. $1.

Examples of string constant escaped ?:

'foo?'
'bar\'d?'
'qux''d?'
'foo said "bar"?'

Quoted Identifiers

A ? that is part of a valid quoted or delimited identifier i.e. surrounded by double quotes is not replaced with a parameter placeholder.

Examples of quoted identifier escaped ?:


"foo?"
"bar said \"foo\"?"
"qux'd?"
"foo said ""bar""?"

None of the question marks in the above examples will be replaced with a parameter placeholder.

Benchmarks

This new version of Rebind() is slower than the previous strings.Index version. However, it is faster than the bytes.Buffer version.

goos: linux
goarch: amd64
pkg: github.com/jmoiron/sqlx
BenchmarkRebind/index-8        	 2114181	       598 ns/op
BenchmarkRebind/new-8         	 2827218	       747 ns/op
BenchmarkRebind/buff-8        	 1000000	      1678 ns/op

Issues

This should fix #487

@dlsniper dlsniper added could merge The PR look safe enough to merge needs testing The PR needs more testing before being accepted labels Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
could merge The PR look safe enough to merge needs testing The PR needs more testing before being accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sqlx.In and regexes containing question mark
4 participants