- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[cli] MySQL Diesel only supports tables with primary keys. #2786
Comments
I cannot reproduce this behaviour with both migrations you've provided. Feel free to add additional information that may help reproducing this problem, like:
USE information_schema;
select column_name, table_name
from key_column_usage
where table_name = 'junk' and constraint_name IN (
select constraint_name from table_constraints where constraint_type = 'PRIMARY KEY'
)
Closed as cannot reproduce. (We will reopen this issue if there is any way to reproduce it) |
I've also now try testing it with Ubuntu thru WSL2 as well as from other Linux machine on the same network and getting the same error so not sure why you can't reproduce. |
Just something else I noticed the generated schema.rs file is completely empty but the |
Hmm, that's strange. That result looks correct. We have tests for this as well, so it's quite surprising to hear that it does not work. It might be useful to run those tests locally to see if they are working as expected for your installation. If that's the case the error would be somehow related to your table definition, if that's not the case we would know that this is a general issue. You can run the tests by doing the following steps:
These test cases contain several similar table constructions, so it seems unlikely that this is not catched there. |
I've also looked at and tries some migrations from an old project where I was using diesel where I know I don't have any problems and they are also showing the same error now with the new cli version so looks like it's some kind of strange regression going on. I'll take a look at try out the tests I'm also looking at a fork I seem to have made around the same time to see if there's something I did there that fixed any issues I might have been having at the time and I just forgot to do a PR for them ;) Once I remember how to bring in the upstream changes I'll see what I find on that front as well. |
Here's a log on master branch test run:
Note that master still says its building v1.4.0 here.
Note because of outdated I'll try quick fix by just changing the
Looks a lot like the Maybe I'd doing something wrong but I don't really think so since some of the tests seem to run but others don't. Let me know if you want me to try something else and also if you want me to try opening some issues for things like the bad Edit: Fixed missing code block start on second run. |
The tests panic because you've likely used a connection with insufficient permissions. Those tests need to be able to create and drop databases.
I'm happy to get suggestions on how to improve our testing story, but I feel there is really not much we can do to ever test all potential environments. |
So now the user is effectively Edit:
|
The panic while panicing is very likely coming from this |
I know for regular use DATABASE_URL includes a actual DB part but on the tests should it have one or does it matter?
|
Did cleanup of some stuff I was trying and switched back to master and I'm getting some other interesting errors now.
running I seem to be finding all kinds of interesting stuff which is really not helping track down the root cause here for us of why my setup breaks vs CI etc. Edit: Never mind just needed to do update from Github and back to the panicking error during tests :) |
Tried commenting out the drop in
So that doesn't seem to help. Other ideas I can try? Edit: BTW tried switching the URL to |
Could you grab a stack trace via gdb and provide exact steps how to trigger this? Otherwise its not possible to say much about this error. |
This was the original problem that I was trying to describe in my issue (then I got very busy for a week so I wasn't able to follow up on it). I am also having this issue. |
@SpBills Just commenting that you have the same issue will not magically fix it. Please provide any details that can help us reproducing the problem. The best approach for this is probably bundling together your migration with some docker container where you can reproduce the issue. |
@weiznich I have now tested it with a MariaDB instance in a docker container. It works correctly. Below are some details. Docker version: 20.10.6, build 370c289 |
@weiznich I have met the same problem when I used mysql:8.0 docker image,but it works correctly when I used mysql:5.7 docker image。 |
@bigtree9307 Please provide a exact version. 8.0 is quite a large version range. |
@weiznich I think it is 8.0.26. The docker image is here https://hub.docker.com/_/mysql?tab=description&page=1&ordering=last_updated |
@bigtree9307 Can you additionally provide the following information:
|
@weiznich |
I recently ran into this using Mysql 8.0.26. The curious thing is that the exact same migration sql, and exact same Some info first:
Tracing backwards from the error location, I see that it uses this to determine the primary keys for each table. After running Corresponding to lines 149-151, we query from
So far, so good. Note that the rust code is only selecting Next, lines 153-156 are fairly straightforward, it's just copying the schema name. Finally, tracing the final query of
So, as far as I can tell, all of the data is there! (Even after the CLI printed that error, FWIW.) At this point, my top candidate place to verify is to check that the |
I now believe this is possibly a race condition of some sort, potentially with the See here how it is not deterministic:
One thing I should note: I'm on ARM. |
@jmqd Thanks for your debugging work 🎉 I think an easy easy way to verify that it is some kind of race condition is to just try using Edit: Reopened this issue, as this seems to be actionable now. |
I seem to be running into this issue. print-schema blows up with primary key issue. I did not have any issues on my dev and staging environment. I created my production database this morning which came up as 8.0.26. (It appears that previous migrations on daily and staging were done on an earlier version. They are now also at 8.0.26, but they do not the issue.). Is there any information I could provide that may help with this issue? |
I didn't dig any deeper into this issue, as it was not deterministic, and I was able to unblock myself by blowing up the migration and retrying until it went through. I believe there are three main possibilities, but I have no leads on which is correct:
|
@zdannar Are you also on an ARM machine? |
I am not on an ARM machine (Mac Intel and reproducible with linux containers) |
My backtrace is produced while it is checking foreign keys. |
@zdannar If you have a reproducible container that would be really helpful. Otherwise, I assume that this is caused by a changed way how mysql updates the information_schema. If it's a purely time based thing, that might be fixable by just adding a small amount of "sleep time" in the right place. Otherwise it could require reopening the connection at all. Testing that would require code modifications and trying out a few variants. If someone is interested in that, this would be a good entry point: Lines 69 to 73 in 4fe9876
I would probably just start with adding a std::thread::sleep_ms(1000) there between line 72 and 73.
Other than that I can say that variant 3 from @jmqd's list is quite likely not the cause, as there is just no parallelism there. We execute the migrations one by one in order and afterwards the schema file is regenerated. |
@weiznich The weird part is that the whole schema is there and looks fine. Tables, primary keys, foreign keys and such appear to be completely in order. |
Just to be sure: Your error happens with |
@weiznich Both. |
Can you provide a backtrace for that? This seems like a different kind of issue. |
RUST_BACKTRACE=full diesel print-schema 101 ✘ 15:05:12 |
So essentially this query is the problem. Can you try to execute the underlying query manually and see what it returns? |
I haven't pulled it all apart yet... If you are just looking at the table in question. ``` mysql> select version(); |
A broader look.
|
I started trying to compile a local cli and add some statements, but haven't gotten very far on the compilation. |
Can you also provide some content of |
|
Sorry, I should have included the select statement for reference.
|
I checked this output against the same schema in my dev/staging databases.. no difference. I have no idea what is going on. |
I turned on the general_log and got exactly what was being sent. It appears that this is a mysql IN clause issue. (I could have made a mistake though).
|
I actually run into this issue a couple of years ago. Trying to find reference to the issue. |
I am not sure how to change information_schema.rs.::get_primary_keys to execute the query like this.
|
Regardless, I am going to try rebuilding my database and see if the information schema is somehow corrupted. |
This would require to change this section: diesel/diesel_cli/src/infer_schema_internals/information_schema.rs Lines 262 to 277 in 4fe9876
to something like this: key_column_usage::table.inner_join(table_constraints::table.on(table_constraints::constraint_name.eq(key_column_usage::constraint_name)))
.filter(key_column_usage::table_name.eq(&table.sql_name))
.filter(key_column_usage::table_schema.eq(schema_name))
.filter(table_constraints::constraint_type.eq("PRIMARY KEY"))
.select(key_column_usage::column_name) (Additionally it is required to adjust the where clause above to make this compile.) |
I rebuilt the database completely and still have the issue. I am having to convert all of my code over to postgres but want to see if I can fix this later tonight. |
I was able to recreate this with existing pg db. You just need to login as a user who lacks privileges. |
@COREjake This issue is about a mysql specific problem. Your comment about postgres specific problems seems to be not right here. Please do not add these off-topic comments to existing issues. |
Sorry if it's like that, but this is the only thread on this I found. My finding was that error is too vague and ambiguous. Without the necessary privilege you can still get the same error and wording will be misleading to say the least. |
Setup
set
DATABASE_UR
with DB server URL.Make new migration which contents:
-- OR --
-- OR --
Any other valid SQL syntax to make a table with a
PRIMARY KEY
Versions
Problem Description
do:
diesel migration run
What is the expected output?
Running migration 2021-05-18-193949_junk
What is the actual output?
Checklist
closed if this is not the case)
The text was updated successfully, but these errors were encountered: