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: incompatible array type names in pg_type (Npgsql support) #13470

Closed
roji opened this issue Feb 7, 2017 · 10 comments
Closed

sql: incompatible array type names in pg_type (Npgsql support) #13470

roji opened this issue Feb 7, 2017 · 10 comments
Assignees
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL
Milestone

Comments

@roji
Copy link

roji commented Feb 7, 2017

Continuing attempts to get Npgsql (.NET PostgreSQL driver) to work with CockroachDB (#9955), we can now connect to the database but can't transfer any types.

The reason is that type names in pg_type have names which have nothing to do with the standard names in PostgreSQL. For example, while PostgreSQL has types int2 (oid 21), int4 (oid 23), int8 (oid 20), in Cockroach all these types have the name int.

To clarify, it seems that most database drivers contains a simple, hardcoded list of OIDs which they recognize. Npgsql, however, queries pg_type upon first connecting and maps by type names - it looks for int4 (not int) and gets its OID.

It could be argued that Npgsql should also load by OIDs (or at least provide an option to do so), but you guys seem to want to be as PostgreSQL-compatible as possible (otherwise you wouldn't have implemented pg_type in the first place). May I suggest you synchronize your pg_type with PostgreSQL as much as possible?

Once this is done I'll be happy to continue testing Npgsql and reporting issues, to the point where you can list it as a compatible driver for C#/.NET.

Jira issue: CRDB-14703

Jira issue: CRDB-14705

@jordanlewis
Copy link
Member

Hi @roji, we're working on this right now - see #13355. We're about to merge a patch that will correct most of these type name mismatches.

@roji
Copy link
Author

roji commented Feb 7, 2017

Great to see @jordanlewis!

Another issue I just stumbled into, is that the pg_type.typreceive field is 0 for all types. Npgsql does an inner join with pg_proc and checks whether the proname is array_recv in order to detect arrays. At the very least pg_type.typreceive needs to contain the OID of an existing row in pg_proc for the join to actually work.

I realize that making CockroachDB 100% compatible with PostgreSQL is a lot of work, although I think once we get past the type loading phase things should be fine. FYI, below is the query Npgsql executes in order to load types when first connecting to a database. Some of it is totally irrelevant for you guys (loading ranges, enums, composites), and I don't think you're far off from this actually working. Please let me know how I can help further.

SELECT ns.nspname, a.typname, a.oid, a.typrelid, a.typbasetype,
CASE WHEN pg_proc.proname='array_recv' THEN 'a' ELSE a.typtype END AS type,
CASE
  WHEN pg_proc.proname='array_recv' THEN a.typelem
  WHEN a.typtype='r' THEN rngsubtype
  ELSE 0
END AS elemoid,
CASE
  WHEN pg_proc.proname IN ('array_recv','oidvectorrecv') THEN 3    /* Arrays last */
  WHEN a.typtype='r' THEN 2                                        /* Ranges before */
  WHEN a.typtype='d' THEN 1                                        /* Domains before */
  ELSE 0                                                           /* Base types first */
END AS ord
FROM pg_type AS a
JOIN pg_namespace AS ns ON (ns.oid = a.typnamespace)
JOIN pg_proc ON pg_proc.oid = a.typreceive
LEFT OUTER JOIN pg_type AS b ON (b.oid = a.typelem)
LEFT OUTER JOIN pg_range ON (pg_range.rngtypid = a.oid)
WHERE
  (
    a.typtype IN ('b', 'r', 'e', 'd') AND
    (b.typtype IS NULL OR b.typtype IN ('b', 'r', 'e', 'd'))  /* Either non-array or array of supported element type */
  ) OR
  (a.typname IN ('record', 'void') AND a.typtype = 'p')
ORDER BY ord

@jordanlewis jordanlewis added the A-sql-pgcompat Semantic compatibility with PostgreSQL label Feb 7, 2017
@jordanlewis
Copy link
Member

Ah, yeah. We provide minimal support for the array_in builtin so that pg_type.typinput can join to array_in's OID for this exact reason - another ORM uses that trick to detect array types.

Thanks for the report - we can do the same thing here. Boy, wouldn't it be nice if pg_type provided a unified way of doing this? Out of curiosity, wouldn't the typcategory column being set to 'A' provide virtually the same functionality?

@jordanlewis
Copy link
Member

That method would exclude the _record type and include further the oidvector and int2vector types.

@roji
Copy link
Author

roji commented Feb 7, 2017

@jordanlewis the typcategory suggestion looks good - I suspect the main reason Npgsql isn't using it is because typcategory was introduced in PostgreSQL 8.4. Note that for oidvector there's already special handling in the Npgsql query - we check for oidvectorrecv as well as for array_recv.

Unfortunately this whole area is a bit messy, and suffers from some backwards-compatibility constraints. IMHO the most natural thing would have been for typtype to contain A... But we have to make do with the existing, so the best thing would be for you guys to get as close as possible to PostgreSQL as possible...

@knz knz changed the title Npgsql support: incompatible type names in pg_type sql: incompatible array type names in pg_type (Npgsql support) Feb 10, 2017
@jordanlewis
Copy link
Member

This is blocked on #13567

@cuongdo cuongdo added this to the Later milestone Feb 22, 2017
@cuongdo
Copy link
Contributor

cuongdo commented Feb 22, 2017

Not scheduled for 1.0 (but may happen depending on other work)

@mattiasmak
Copy link

A similar issue happened to me when trying to use Microsoft Power BI with npgsql.

Details: "An error happened while reading data from the provider: 'The field 'TABLE_SCHEMA' has a type currently unknown to Npgsql (OID 25). You can retrieve it as a string by marking it as unknown, please see the FAQ.'

I guess this is not something you can fix, but it may be good to know.

@jordanlewis
Copy link
Member

Hey @roji,

I think #14529 should have solved your issue here - we added the type I/O functions to pg_type and pg_proc. Could you please take a look at your sample app at your leisure and see if all is well?

@BramGruneir BramGruneir self-assigned this Oct 11, 2017
@BramGruneir BramGruneir modified the milestones: Later, 1.2 Oct 11, 2017
@BramGruneir
Copy link
Member

Closing this issue, if there are any issues, please reopen or create a new issue.

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

5 participants