-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: pgwire OID error during EF Core / Npgsql migration #24173
Comments
Hi @rkdrnfds, I don't understand this one. Why does |
Can you guys provide some more info? Which version of Npgsql is being used, and when did this start occurring - on upgrade of an Npgsql version (from which to which), an upgrade of Cockroach... I'm going to assume this is a Cockroach-side issue for now (since PostgreSQL doesn't seem to be affected), but I'll monitor this issue in case you guys need help/guidance. FYI the upcoming Npgsql 4.0 will have an extensible type loading API, which allows you to bypass pg_type entirely and load types your own way (or just hardcode them) - take a look at npgsql/npgsql#1486 (comment). Full PostgreSQL compatibility via pg_type (and related tables) is still the best possibility, but this could be relevant for Cockroach. Let me know if you need more details. |
This occured under When upgraded cockroach from 201801xx to 20180319 version. Additionally, I'm not experiencing this problem since using cockroach 2.0 stable version. |
I've only been using the Oops: and |
I'm keen to help if I can, it's a bit of a blocker for me. Would it be possible to establish where the issue lies? (I also get the issue on To me it sounds like the Npgsql must be assigning an Unknown type handler for this type for some reason. Am I right in thinking it's either a broken implementation of bools in the wire protocol from CockroachDB, or something about setting up mappings in Npgsql is misinterpreting something? Can anyone share any hints for how to diagnose this? |
FYI for the Npgsql side I'm away for the next 1.5 weeks or so but will be back and available to discuss |
@kierenj it'd be great if you could start your app and get a log of all the metadata queries that get sent to the database. That usually helps narrow things down. You can start cockroach with |
Ok cool, here we are. Note that I'm pretty much following https://www.cockroachlabs.com/docs/stable/start-a-local-cluster-in-docker.html#os-windows, though only 2 nodes, called (Running that first query which is looking for types manually does return a row for
|
Yeah, I still don't get it. Running that query against CockroachDB and against Postgres produces identical results for the bool row. I also doubt that it has to do with the wire protocol. Things like this are usually caused by subtle catalog entry differences, or faulty assumptions on the driver side. CockroachDB doesn't guarantee that all catalog OIDs are the same as Postgres for most of these virtual tables (pg_type is an exception), but we do keep the association between tables intact. So, if a driver assumed that other OIDs existed, it'd be an error with us. That doesn't seem like it's happening here, but it probably bears mentioning. If I were more familiar with dot net, I'd probably pop open a debugger and figure out why the mapping isn't getting set up properly. |
Ah, thanks. I may try to do just that. Really keen on getting my all-kube megademo going ;) |
Note that Npgsql doesn't actually care what type OID is exposed, it looks for the type name ( |
@roji the
...and the same for In (Continuing to dig in..) ...because
@jordanlewis any way of diagnosing responses to queries, not just logging them coming in? |
@kierenj this is definitely not normal and explains the errors you're seeing. I'd suggest a Wireshark session to look at exactly what's going back and forth between Npgsql and Cockroach. My guess is that some incompatibility between Cockroach and npgsql's type loading query is causing the empty results (probably one of the joins). The best way to isolate this is simply to get the type loading query's SQL and execute it outside Npgsql, and then play around with it to find the source of the incompatibility. As I wrote above, the next version of Npgsql (4.0, out in a couple months) allows plugins to define a custom type loading mechanism. If full PostgreSQL compatibility wrt pg_type and friends is too difficult this could be a good option moving forward. |
@roji cool. I ran the query by pasting it in and it seemed to give good results (via the text interface cockroachdb provides). The differences would be that 1) I'm using that text interface and 2) I'm not sure if Wireshark will work to intercept traffic going into a docker container, but I'll see what I can do. ( @jordanlewis if there's more logging for responses I can turn on, or @anyone if you know an intercepting TCP logging proxy whose output would be in a good enough format to you guys in the know, let me know. Tyvm :) ) |
Ok, I have a pcap file :) https://www.dropbox.com/s/rfqhyerdnzqbreq/issue-24173.pcap?dl=0 In Wireshark, packet number 48 seems to be the query heading in. Hex dump of the TCP convo:
|
Thanks! It sure looks like nothing's coming back from the server. Is there any way you could make a small proof of concept app that demonstrates this behavior that I could play around with myself? Someone made an attempt here #24172, but I couldn't observe the issue using that sample. |
Ok - this does it for me. I made it as minimal as possible (1 code file with 2 lines of code, 1 project file with 1 dependency) :) https://github.com/kierenj/cockroach-24173-repro Is there anything else I can provide? Oh I guess in terms of the cockroach environment: as I mentioned it's the 2.0.0 docker image, using (PowerShell syntax under Windows):
Then:
Setup consists of |
I'm not pushing, but just curious - the 2.1 milestone, is there a rough timeline on the roadmap for it? |
We have made significant strides on this area of the code in CockroachDB 2.1. We would appreciate if you could attempt to reproduce the issue with the latest 2.1 version and report your results. Thanks |
@knz I don't see any 2.1 docker image, so I can't repro in the same way I did before. But the git repo I supplied is very straightforward to run - if you have a 2.1 running, it might be easier that way? Else, is there somewhere you can point me for a 2.1 image please? |
@kierenj we don't currently have an environment ready for windows testing -- this will come later in the release cycle. However you can find a docker image for cockroachdb 2.1 here: https://hub.docker.com/r/cockroachdb/cockroach-unstable/ Cheers |
Its not windows-specific? |
.NET testing falls under the windows testing umbrella for us |
Ill give it a go |
Unfortunately I get a very similar exception still:
This looks to be the same case as above, so hopefully the packet capture above is still handy :) |
Hey there @kierenj, Thanks again for the repro. This issue is solved in 19.1, and your repro case seems to execute successfully. I downloaded
So, going to close this out. Please re-open if you have further issues! |
When used ef core cli commands
dotnet ef database update
to auto generate database tables,
it generates error as below.
v2.0-alpha.20180129 did not generate those error when doing the same job, so I guess there's some compatibility issue in v2.0-beta.20180319 with postgres driver(Npgsql).
The text was updated successfully, but these errors were encountered: