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: support array_in builtin function #12139

Closed
jordanlewis opened this issue Dec 7, 2016 · 3 comments
Closed

sql: support array_in builtin function #12139

jordanlewis opened this issue Dec 7, 2016 · 3 comments
Assignees
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL

Comments

@jordanlewis
Copy link
Member

jordanlewis commented Dec 7, 2016

ActiveRecord attempts to look up OID of the array_in builtin function to search pg_type for entries with a matching typinput field in the query below. We don't support this builtin function, but we need to make an effort to do so to support ActiveRecord.

SELECT t.oid, t.typname, t.typelem, t.typdelim, t.typinput, r.rngsubtype, t.typtype, t.typbasetype
FROM pg_type as t
LEFT JOIN pg_range as r ON oid = rngtypid
WHERE t.typname IN ('int2', 'int4', 'int8', 'oid', 'float4', 'float8', 'text', 'varchar', 'char', 'name', 'bpchar', 'bool', 'bit', 'varbit', 'timestamptz', 'date', 'time', 'money', 'bytea', 'point', 'hstore', 'json', 'jsonb', 'cidr', 'inet', 'uuid', 'xml', 'tsvector', 'macaddr', 'citext', 'ltree', 'interval', 'path', 'line', 'polygon', 'circle', 'lseg', 'box', 'timestamp', 'numeric')
OR t.typtype IN ('r', 'e', 'd')
OR t.typinput = 'array_in(cstring,oid,integer)'::regprocedure
OR t.typelem != 0

array_in is pretty much entirely undocumented. It's mentioned in the Create Type documentation and this random ebook. It seems like it's an "input function" that basically parses an array into some Postgres internal format.

@jordanlewis jordanlewis added the A-sql-pgcompat Semantic compatibility with PostgreSQL label Dec 7, 2016
@jordanlewis
Copy link
Member Author

This task exposes more deficiencies in our pg_catalog implementation.

  1. Casting with ::regprocedure actually has different behavior than ::regproc: it requires that the input function name come with a type parameter list. Currently, we treat ::regprocedure the same as ::regproc.
  2. We don't support the cstring, oid, or integer types by those names.

We could make the function lookup we implement for ::regprocedure ignore the type parameter list entirely, which would strip ::regprocedure of its disambiguating functionality in the face of overloaded functions. This probably would be fine as I don't expect that the average user will be using the ::regprocedure cast, but it would certainly be a disingenuous implementation.

Or, we could add type aliases for cstring, oid and integer, either in a fully-fledged way or just in the ::regprocedure implementation.

@nvanbenschoten, do you have any thoughts on this w.r.t. your recent work on type aliasing?

@jordanlewis jordanlewis self-assigned this Jan 17, 2017
@nvanbenschoten
Copy link
Member

There are a few interesting things to note about the line t.typinput = 'array_in(cstring,oid,integer)'::regprocedure:

  1. We currently do not populate pg_type.typinput. We'll need to, at least partially.
  2. All array_in procedures have the same oid, so the line could be written as t.typinput = 'array_in'::regproc and have the same effect. This means that the "disambiguating functionality" isn't actually used at all in the query.

I think your first suggestion is the approach we should take here. Until the disambiguation is proven to be necessary, I don't think it's worth supporting.

I actually think the bigger issue is supporting these typinput functions, which are internal, undocumented Postgres-specific functions. I personally have only ever seen them used when looking for array-like types by doing exactly what this query is doing (i.e. looking for all types that use the array_in constructor). Perhaps that's all we need to support.

@jordanlewis
Copy link
Member Author

Good point about the pointless use of ::regprocedure. I notice now that there is in fact only a single procedure named array_in. I think you're right that we can just leave the "disambiguation" bit unimplemented.

I agree that we don't really need to implement array_in. I think we could add a stub builtin that returns an unimplemented error just for this purpose. I think we can also just populate typinput for those array-like types like you're saying. Examining the file postgres/oid/type_map_initializer.rb in Rails shows that the typinput result column of this query is only used to compare against the string array_in, on this line.

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
Projects
None yet
Development

No branches or pull requests

3 participants