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.

Sign up for GitHub

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

Jump to bottom

sql: query issued by jdbc doesn't typecheck #17140

Closed
justinj opened this issue Jul 20, 2017 · 0 comments
Closed

sql: query issued by jdbc doesn't typecheck #17140

justinj opened this issue Jul 20, 2017 · 0 comments
Assignees
@jordanlewis @justinj
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Milestone
1.1

Comments

@justinj
Copy link
Contributor

justinj commented Jul 20, 2017 •
edited

Below is a simplification of the original query:

PREPARE foo (INT) AS SELECT e.typdelim FROM pg_catalog.pg_type AS t WHERE (t.oid = $1);
pq: unsupported comparison operator: <oid> = <int>
@justinj justinj added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jul 20, 2017
justinj pushed a commit to justinj/cockroach that referenced this issue Jul 20, 2017
sql: improve string formatting for postgres-compat
60f1743 
This change introduces a new method of formatting strings within arrays
which is more compatible with how Postgres formats them. It's also a
slightly simplified version of what Postgres does. The primary
difference is that we quote every string, and we use double quotes
instead of single quotes.

These changes are required for the node.js postgres driver.

I added tests to the python and node acceptance tests.
The ruby pg gem doesn't seem to support arrays at all, and adding a java
one is blocked on cockroachdb#17140.

I ran a quick benchmark comparing the two functions:

```
func BenchmarkEncodeSQLString(b *testing.B) {
	str := strings.Repeat("foo", 10000)
	b.Run("old version", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			encodeSQLStringWithFlags(bytes.NewBuffer(nil), str, FmtBareStrings)
		}
	})
	b.Run("new version", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			encodeSQLStringInsideArray(bytes.NewBuffer(nil), str)
		}
	})
}
```

and the results are that the new version is substantially faster, though
this isn't entirely fair since there are no escaped characters in the
benchmark, so the old version does a lot of unnecessary work. It is
reassuring that there isn't much, if anything, lost due to inserting
into the buffer char-by-char using WriteByte instead of using
WriteString.:

BenchmarkEncodeSQLString/old_version-8  2000 1035068 ns/op
BenchmarkEncodeSQLString/new_version-8  5000 304645 ns/op
@justinj justinj added this to the 1.1 milestone Aug 9, 2017
@jordanlewis jordanlewis mentioned this issue Aug 23, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jordanlewis jordanlewis

@justinj justinj

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
1.1
Development

No branches or pull requests
2 participants
@jordanlewis @justinj