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

sql: auto array type cast from '{}' in column default definition for better pg compatibility #33341

Closed
kimxogus opened this issue Dec 24, 2018 · 3 comments · Fixed by #38869
Closed
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-typing SQLtype inference, typing rules, type compatibility. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community

Comments

@kimxogus
Copy link

Is your feature request related to a problem? Please describe.
I'm using scala backend and https://github.com/slick/slick and https://github.com/tminglei/slick-pg, and I'm trying to use cockroach with them.

slick-pg generates sql like below with array column defaults.

create table test {
  foo text[] default '{}'
}

In postgres, that SQL works.

But in cockroach, Error: pq: expected DEFAULT expression to have type string[], but ''{}'' has type string occurs and sql should be fixed as below.

create table test {
  foo text[] default '{}'::text[]
}

Describe the solution you'd like
Automatically cast '{}' to its own column type.

Describe alternatives you've considered
Modify https://github.com/tminglei/slick-pg to generate sql with type cast like ::text[], but I think fixing in cockroach would be better in pg-compatibility.

Additional context

@knz knz added A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-typing SQLtype inference, typing rules, type compatibility. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community labels Dec 24, 2018
@knz
Copy link
Contributor

knz commented Dec 24, 2018

This overlaps with, but is not fully duplicate of, #23299

@khalibartan
Copy link
Contributor

@jordanlewis Can you help me getting started with this one?

@jordanlewis
Copy link
Member

This issue requires adding some functionality to the "constant up-cast" system in CockroachDB. This subsystem lives in pkg/sql/sem/tree/constant.go. Its job is to take constants, which can either be numeric or string-like, and resolve them into real types in the type system.

The trouble in this case is that we don't have a way to upcast a string into an array. The relevant code here is StrVal.ResolveAsType in the file I mentioned above. You'll need to add a type switch for array types in the second half of that function, and reuse the array parsing code from pkg/sql/sem/tree/parse_array.go (ParseDArrayFromString) to actually get an array type out from the string.

Does that help?

craig bot pushed a commit that referenced this issue Jul 19, 2019
38869: sql: compatibility blitz for pgadmin r=jordanlewis a=jordanlewis

A bunch of small improvements to the catalog, new builtins, and a couple of typing improvements all in the name of making pgadmin work.

And it works okay now! Definitely still some flaws and unsupported things, but at least the default experience isn't completely broken anymore.

Closes #33341.
Closes #23299.
Closes #26389.
Closes #26378.
Closes #26390.
Closes #24747.
Closes #37124.
Updates #25213.

![image](https://user-images.githubusercontent.com/43821/61190353-f17fb980-a668-11e9-947f-d1bc3bb1e75d.png)

Co-authored-by: Jordan Lewis <[email protected]>
@craig craig bot closed this as completed in #38869 Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-typing SQLtype inference, typing rules, type compatibility. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants