dolthub / dolthub-issues

Issues for dolthub.com
https://dolthub.com
4 stars 1 forks source link

unexpected behaviour when dropping tables #541

Closed ShaunSHamilton closed 4 months ago

ShaunSHamilton commented 4 months ago

Admittedly, our workflow is odd, because we are still in the playing/testing phase of using Dolt. So, some of this could be our fault.

Workflow

1) local: drop all tables in database 2) local: run script populating database

Issue

The dropped tables do not get dropped on the upstream; instead, they are duplicated.

Here is what the Dolthub diff looked like for one such PR ![image](https://github.com/dolthub/dolthub-issues/assets/51722130/8e6e103a-8116-412d-a235-d07c96a0ae7a)

The camelCase named tables were dropped. I paired with a colleague on this, and we are both 100% sure, locally, they were dropped. Then, the script created a bunch of tables (this time in snake_case), and this was added, committed, and pushed to be opened in a PR.

We thought the diff was just a bit wonky. So, merged the PR. Then, we noticed the diff was not necessarily wonky, but the dropped tables were not removed as expected:

Image of `main` ![image](https://github.com/dolthub/dolthub-issues/assets/51722130/236678bc-c773-482d-81cd-32b1d0216ffb)

Expected Behaviour

The diff, and subsequent merge should have removed the camelCase named tables, and the snake_case named tables should have been added.

I was not 100% sure where to open this issue. I noticed it in Dolthub, because of the diff view, but please move it.

liuliu-dev commented 4 months ago

Hi @ShaunSHamilton , could you give an example of the workflow or the scripts you ran? It will help us reproduce the bug.

I was able to drop table and merge the changes into main, could you provide me with the script populating database after dropping tables?

Thank you!

ShaunSHamilton commented 4 months ago

@liuliu-dev Sorry for the mess: https://github.com/ShaunSHamilton/freeCodeCamp/blob/dolt/dolt/index.js 🙈

Let me know if anything more could help.

liuliu-dev commented 4 months ago

Hi @ShaunSHamilton thanks for the file.

I could not repro the bug. These are the steps i did:

this is the diff page of the pull request:

Screenshot 2024-04-24 at 10 50 29 AM

the diff looks correct and after merge job finished, the tables are dropped.

Could you give me more details of your workflow to help me identify the issue? Thanks.

ShaunSHamilton commented 4 months ago

I am not too sure what more to say.

To clarify, I never adjusted the constraints of any table. The workflow is always: 1) drop ALL tables 2) run script to repopulate database 3) commit + push

My image showing the constraint diff is like that, I assume, because I dropped the table and recreated it.

liuliu-dev commented 4 months ago

Hi @ShaunSHamilton ,

Just a few updates:

I've submitted a pull request to fix the table name list in the diff overview. I expect to merge it either today or tomorrow.

Also, I've successfully replicated the merge issue by cloning your database, but I'm still investigating the cause. I'll keep you posted. Apologies for the inconvenience.

liuliu-dev commented 4 months ago

Hi @ShaunSHamilton , do you still have branch feat/update-tables locally on database mot01/curriculum ? could you check if the camelCase named tables are dropped there? Thank you!

ShaunSHamilton commented 4 months ago

@moT01 ☝️

moT01 commented 4 months ago

I didn't still have the branch locally. I don't quite have my dolt workflow figured out - or I'm not doing it right or something. I have to reclone my fork often. My guess is that it has something to do with git. I have my dolt database folder inside a folder that's a git repo. So when I am switching git branches, and doing other git things, the dolt references get lost or something? There's still a .dolt folder there, but dolt status gives me "The current directory is not a valid dolt repository." So I try to reclone, and I get this: "data repository already exists: ." - so I delete the .dolt folder and reclone it.

When I did that, and checkout my feat/update-tables branch, the tables do still all exist:

images Screenshot 2024-04-26 at 8 36 03 AM
Screenshot 2024-04-26 at 8 36 11 AM
timsehn commented 4 months ago

You need to add .dolt to .gitignore or the actual files in the Dolt database will get added to your Git repo. So when you switch branches in Git things will get messed up.

liuliu-dev commented 4 months ago

@moT01 Thanks for checking.

It appears that the camelCase-named tables were not dropped on the feat/update-tables branch, which explains why the diff still shows them and why they remain after the merge.

ShaunSHamilton commented 4 months ago

Thank you for your time looking into this. I have not experienced this since. So, will close the issue until if comes up again.