sql: Relax conversion rules in non-lossy cases for greater PG compatibility

[andy-kimball]

Suggestion

In cases where a conversion is non-lossy and non-ambiguous, we should consider doing an implicit conversion. For example:

CREATE TABLE t (a STRING);
INSERT INTO t (a) VALUES (123);

Converting from Int to String is not lossy, and in the context the conversion is not ambiguous (i.e. it's not a case like SELECT '01' = 1). Doing this will give CRDB greater compatibility with PG without compromising type safety.

Use case

When using the off-the-shelf ActiveRecord Postgres driver, I saw the following error when invoking rake db:reset:

Caused by:
PG::DatatypeMismatch: ERROR:  value type int doesn't match type varchar of column "version"
/Users/andyk/.rvm/gems/ruby-2.6.0/gems/activerecord-5.2.3/lib/active_record/connection_adapters/postgresql/database_statements.rb:75:in `async_exec'
/Users/andyk/.rvm/gems/ruby-2.6.0/gems/activerecord-5.2.3/lib/active_record/connection_adapters/postgresql/database_statements.rb:75:in `block (2 levels) in execute'
/Users/andyk/.rvm/gems/ruby-2.6.0/gems/activesupport-5.2.3/lib/active_support/dependencies/interlock.rb:48:in `block in permit_concurrent_loads'
/Users/andyk/.rvm/gems/ruby-2.6.0/gems/activesupport-5.2.3/lib/active_support/concurrency/share_lock.rb:187:in `yield_shares'
/Users/andyk/.rvm/gems/ruby-2.6.0/gems/activesupport-5.2.3/lib/active_support/dependencies/interlock.rb:47:in `permit_concurrent_loads'
/Users/andyk/.rvm/gems/ruby-2.6.0/gems/activerecord-5.2.3/lib/active_record/connection_adapters/postgresql/database_statements.rb:74:in `block in execute'

It turns out the reason is because ActiveRecord generates the following SQL, even though version is a varchar column:

INSERT INTO schema_migrations(version) VALUES (20190503234707);

[jordanlewis]

Interestingly, Postgres doesn't permit a variation on this idea:

jordan=# select case 'one' when 1 then 1 when 'two' then 2 end;
ERROR:  operator does not exist: text = integer
LINE 1: select case 'one' when 1 then 1 when 'two' then 2 end;
                          ^
HINT:  No operator matches the given name and argument types. You might need to add explicit type casts.

This example would be permitted if we were to simply permit quote-less numeric constants to be coerced to string. This feels okay to me, but it's worth considering why Postgres (and it's generally permissive type system) doesn't permit this case.

[jordanlewis]

Converting from Int to String is not lossy, and in the context the conversion is not ambiguous (i.e. it's not a case like SELECT '01' = 1). Doing this will give CRDB greater compatibility with PG without compromising type safety.

@andy-kimball, the case you mentioned would actually type-check just fine given this proposal: the 1 would be coerced into a string '1', producing SELECT '01' = '1', and return false. Would you expect that?

Implementing a subset of this proposal is a possibility (maybe only in VALUES clauses or something?) but it seems kind of hard. I have a simple patch that permits numeric constants to become strings, but it does it everywhere, which leads to these issues. #38204

[jordanlewis]

@rohany maybe you would be interested in trying this out, given your recent adventures with the typechecker? This is directly related the "constant upcast" phase I was telling you about.

[github-actions]

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
5 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

[misiek08]

Any news here? Is there anything we can do to help with resolving this issue?

[mgartner]

The first example in this issue will succeed in the next major release, v22.1, now that assignment casts have been implemented.

The example given by @jordanlewis above is not yet supported because it is an implicit cast. See the implicit cast meta-issue #75101, and the issue specific to CASE expressions #75103.

I'll close this issue.