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

PostgreSQL divergence: pg_class entries do not always have equivalent representation in pg_type #66576

Closed
benjie opened this issue Jun 17, 2021 · 11 comments · Fixed by #66815
Closed
Assignees
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-graphile Issues relating to graphile compatibility C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community X-blathers-triaged blathers was able to find an owner

Comments

@benjie
Copy link

benjie commented Jun 17, 2021

Describe the problem

From the Postgres docs:

A composite type is automatically created for each table in the database, to represent the row structure of the table. It is also possible to create composite types with CREATE TYPE AS.
-- https://www.postgresql.org/docs/current/catalog-pg-type.html

These automatically created types are not represented in Cockroach.

To Reproduce

Create the following table (in the public schema):

create table colors (
  id serial primary key,
  name text not null
);

Then run the following:

[graphile_example] # select oid from pg_class where relnamespace = 'public'::regnamespace and relname = 'colors';
┌─────┐
│ oid │
├─────┤
│  77 │
└─────┘
(1 row)

[graphile_example] # select * from pg_type where typrelid = (select oid from pg_class where relnamespace = 'public'::regnamespace and relname = 'colors');
(0 rows)

Notice that there's no pg_type entry for this pg_class.

Expected behavior

Postgres automatically creates the type:

select * from pg_type where typrelid = (select oid from pg_class where relnamespace = 'public'::regnamespace and relname = 'colors');
┌─[ RECORD 1 ]───┬─────────────┐
│ oid            │ 122463      │
│ typname        │ colors      │
│ typnamespace   │ 2200        │
│ typowner       │ 16388       │
│ typlen         │ -1          │
│ typbyval       │ f           │
│ typtype        │ c           │
│ typcategory    │ C           │
│ typispreferred │ f           │
│ typisdefined   │ t           │
│ typdelim       │ ,           │
│ typrelid       │ 122461      │
│ typelem        │ 0           │
│ typarray       │ 122462      │
│ typinput       │ record_in   │
│ typoutput      │ record_out  │
│ typreceive     │ record_recv │
│ typsend        │ record_send │
│ typmodin       │ -           │
│ typmodout      │ -           │
│ typanalyze     │ -           │
│ typalign       │ d           │
│ typstorage     │ x           │
│ typnotnull     │ f           │
│ typbasetype    │ 0           │
│ typtypmod      │ -1          │
│ typndims       │ 0           │
│ typcollation   │ 0           │
│ typdefaultbin  │ ¤           │
│ typdefault     │ ¤           │
│ typacl         │ ¤           │
└────────────────┴─────────────┘

Environment:

  • CockroachDB version: CCL v21.2.0-alpha.00000000-1590-gf56208a85e @ 2021/06/17 05:54:54 (go1.15.11)
  • Server OS: Ubuntu 21.04
  • Client app: psql (Postgres v13)
@benjie benjie added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jun 17, 2021
@blathers-crl
Copy link

blathers-crl bot commented Jun 17, 2021

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Jun 17, 2021
@rafiss rafiss added A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-graphile Issues relating to graphile compatibility labels Jun 17, 2021
@rafiss
Copy link
Collaborator

rafiss commented Jun 17, 2021

Thanks @benjie -- we'll look into it and see what we can do here. Out of curiosity, how does Graphile use the pg_type info for table names? I'm curious if we could get away with a limited subset of info.

@benjie
Copy link
Author

benjie commented Jun 18, 2021

We use the type in general to refer to the composite type since that's what referenced from function arguments and similar situations. I think we generally just use it for the name of the type for casting purposes, determining array variants, etc. The most intense usage of pg_type is in this function:

https://github.com/graphile/graphile-engine/blob/726c210ae6f559c132f859fed18dc031a2a2cb1c/packages/graphile-build-pg/src/plugins/PgTypesPlugin.js#L729

Note we alias the various pg_type columns; you can see that in our introspection query here:

https://github.com/graphile/graphile-engine/blob/726c210ae6f559c132f859fed18dc031a2a2cb1c/packages/graphile-build-pg/src/plugins/introspectionQuery.js#L233-L317

I think the most important columns for us are:

  • oid
  • typname
  • typnamespace
  • typtype
  • typcategory
  • typrelid
  • typenotnull
  • typelem (for arrays)
  • typlen (for arrays)
  • typbasetype (for domains)
  • typtypmod
  • typdefaultbin

@rafiss
Copy link
Collaborator

rafiss commented Jun 21, 2021

@otan could you pick this one up? here's a few details that might make this a little tricky:

  • the automatically created composite type will need to get a new deterministic OID, because there's an index on pg_type.oid.
  • note that pg_type.typrelid is a foreign key to pg_class.oid for these automatically created composite types. and also, pg_class.reltype is a foreign key to pg_type.oid
    -- i wonder if we could get away with making pg_type.oid the same as pg_type.typrelid for these composite types.
  • each automatically created composite type also gets an array type. for example:
rafiss@127:postgres> select 't'::regclass::oid;
+-------+
| oid   |
|-------|
| 34511 |
+-------+

rafiss@127:postgres> select oid, typname, typrelid, typarray from pg_type where typrelid = 34511;
+-------+-----------+------------+------------+
| oid   | typname   | typrelid   | typarray   |
|-------+-----------+------------+------------|
| 34513 | t         | 34511      | 34512      |
+-------+-----------+------------+------------+

rafiss@127:postgres> select oid, typname, typrelid, typarray from pg_type where oid = 34512;
+-------+-----------+------------+------------+
| oid   | typname   | typrelid   | typarray   |
|-------+-----------+------------+------------|
| 34512 | _t        | 0          | 0          |
+-------+-----------+------------+------------+

@otan
Copy link
Contributor

otan commented Jun 22, 2021

This also means that casts to ::regtype for tables should also return the same entry.

Currently, ::regclass makes the OID the descriptor ID. What do we want to do for ::regtype, which we don't allocate IDs for?

@otan
Copy link
Contributor

otan commented Jun 22, 2021

i guess it can be 10000+descriptor_id, like we do for user defined types

@rafiss
Copy link
Collaborator

rafiss commented Jun 22, 2021

Out of curiosity, what happens if we make pg_type.oid == pg_type.typrelid == pg_class.oid == pg_class.reltype for these automatically created composite types?

@otan
Copy link
Contributor

otan commented Jun 22, 2021

well oids are basically meaningless anyway, since a pg_type oid and a pg_class oid may represent completely different things :\

so with that grim analysis, the least we can do is make pg_type.oid unique, i.e. 10000 + descriptor_id. typrelid should point to the descriptor id (since it is pg_class). however, the array type id is a problem as it can collide as we only allocate 1 id for a table, but we need to allocate 2 ids for a type to include the allocated array type :\

@otan
Copy link
Contributor

otan commented Jun 22, 2021

We can also kick the can down the road for pg_class array types by not giving them an array type. But I'm not sure how many ORMs rely on every type having an array type...

The docs seem to suggest it is possible to not have an array type:

typarray oid (references pg_type.oid)
If typarray is not 0 then it identifies another row in pg_type, which is the “true” array type having this type as element

@otan
Copy link
Contributor

otan commented Jun 23, 2021

We use the type in general to refer to the composite type since that's what referenced from function arguments and similar situations. I think we generally just use it for the name of the type for casting purposes, determining array variants, etc.

@benjie i'm trying to understand why the pg_class in pg_type is helpful to you here. Unfortunately there's no really clean way of representing pg_class as pg_type as we did not reserve enough OIDs. Can you determine when this data is useful?

In particular, i'm trying to gauge whether the array variant is required for you. e.g. if I create a table t, do you require pg_type contain both t and _t?

@benjie
Copy link
Author

benjie commented Jun 24, 2021

Postgres generally refers to the type e.g. for function args, function return types, column types, domains, etc, so we use it quite heavily. I think we only need the array variant if an array variant is represented in the database somewhere. Mostly we just read what "types" things are from Postgres (e.g. function arguments) and then we look up those types in the introspection results. If you don't have user defined functions and none of your built in functions can either accept or return an array of a composite type then I think we're okay for the list type?

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-tools-graphile Issues relating to graphile compatibility C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community X-blathers-triaged blathers was able to find an owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants