Cockroach: sql: string concatenation operator stricter than with postgres

[Luit]

Describe the problem

When running Grafana's migrations (with the postgres database type against CRDB) a string concatenation which works on postgres fails on CRDB. The part that breaks is concatenating '' and an integer column.

To Reproduce

What I did:

docker run --rm -d --name roach cockroachdb/cockroach:v2.1.4 start --insecure
docker run --link roach --rm -ti -e "GF_DATABASE_URL=postgres://root@roach:26257/defaultdb?sslmode=disable" --name grafana grafana/grafana

With the second command repeated a few times, while 'fixing' the DROP INDEX migrations by manually dropping the index with CASCADE and recreating a similarly named index without any constraints.

A simpler case: run any CRDB and issue a query like:

select 'Value: ' || 42;

The result of that query is the following:

[email protected]:26257/defaultdb> select 'Value: ' || 42 as v;
pq: unsupported binary operator: <string> || <int>

Expected behavior

[email protected]:26257/defaultdb> select 'Value: ' || 42 as v;
      v      
+-----------+
  Value: 42  
(1 row)

Time: 483.697µs

Environment:

• CockroachDB version 2.1.4
• Server OS: Linux (in Docker)
• Client app: cockroach sql or grafana migrator

Additional context
This is one of the incompatibilities holding back use of Grafana on CRDB. (the two others are DROP INDEX needing CASCADE for UNIQUE constraints, and ALTER COLUMN lowering a string field's size)

[Luit]

PostgreSQL docs detailing valid string concatenation: https://www.postgresql.org/docs/9.4/functions-string.html

non-string || string and string || non-string work for PostgreSQL, but not for CRDB.

[Luit]

Just for the heck of it I was interested in how this part of a database works, and I'm surprised how simple this is all put together. I'm considering just making a pull request if I can find out how to properly find out how to create the BinOp values for this.

I wouldn't mind seeing someone else implement this either, I don't mind spoilers.

[jordanlewis]

We'd love your contributions. Please feel free to chime in with questions here if you've got them. As you identify, it shouldn't be too much more work than to add another BinOp implementation under Concat in eval.go. There might be some more munging that has to occur after that, but you might as well try it out.

Check out CONTRIBUTING.md for instructions to build and test. You can add tests to the concat file in pkg/sql/sem/tree/testdata/eval/concat, which is pretty anemic at the moment. Those tests are runnable with make test PKG=./pkg/sql/sem/tree.

Again, don't hesitate to ping with questions.

[Luit]

Built a naïve first attempt, and querying select 42 || 'test' or select 42::text || 'test' shows quite similar performance. I'm using PerformCast to get the right type, is that the right way to go?

[knz]

I'm using PerformCast to get the right type

PerformCast is the right way to convert a value; however we need to see specific to determine whether you're using it right.

[knz]

Note however that adding overloads on operators may create ambiguity on placeholder type resolution. We'll want to see a run of various client test suites, for example the Hibernate test suite, to get confidence that we don't see regressions in client compatibility.

[Luit]

I'm struggling a bit with this one: in pkg/sql/sem/tree/testdata/eval/concat there's the following test case:

eval
b'hello' || 'world'
----
'\x68656c6c6f776f726c64'

If I query SELECT b'hello' || 'world'; I get helloworld, not that Bytes string (is there difference in eval type and execution type?).

If I ask postgres what select 'hello'::bytea || 'world'; is, I get that in bytea output. select 'hello'::bytea || 'world'::text; on the other hand, is \x68656c6c6fworld, type TEXT.

I guess my problem is in coercion and casting. I guess I should let it coerce to Bytes if possible (if not a strongly typed string ... how do I know?) and still implement || for any explicit concatenations...

This shit's harder than it looks 👀

[knz]

@Luit notice you wrote "if I query ... I get ..."

How do you "get" this result? cockroach sql is notorious for not rendering byte array values well when it should. This is a problem in the SQL client, not in the computation. Check this out:

kena@kenabook ~/cockroach % psql -h localhost -p 26257 -U root
psql (10.6 (Ubuntu 10.6-0ubuntu0.18.10.1), server 9.5.0)
Type "help" for help.

root=#  SELECT b'hello' || 'world';
        ?column?
------------------------
 \x68656c6c6f776f726c64
(1 row)

Note: I am running psql (Postgre' own client program) against a CockroachDB server. The value is what you expect. It's the printing of the byte array by cockroach sql that's broken -- we have a bug open for this.

In general, don't rely on cockroach sql to evaluate your work. Also, double-check the type of the output with pg_typeof(...).

[Luit]

That explains _a lot_. I'll use psql for now, thanks. I did indeed query CRDB using cockroach sql and postgres using psql.

[Luit]

I'm not sure why this was closed... has it been fixed elsewhere? Or is this a won't fix?

[jordanlewis]

Hey @Luit, apologies - I thought your issue had been resolved based on the conversation at the end. I'll re-open this. Note #37311 which I think will solve this case.