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

Merge not showing correct statistics for rows merged #4220

Closed
alecstein opened this issue Aug 26, 2022 · 5 comments
Closed

Merge not showing correct statistics for rows merged #4220

alecstein opened this issue Aug 26, 2022 · 5 comments
Assignees
Labels
new format Issues related specifically to the __DOLT__ storage format

Comments

@alecstein
Copy link
Contributor

alecstein commented Aug 26, 2022

To repro, clone the us-housing-prices-v2 database and run the following commands:

dolt reset --hard vlr4co3jnmsomqdimfghqto0948c8q24
dolt remote add captainstabs https://doltremoteapi.dolthub.com/captainstabs/us-housing-prices-v2
dolt fetch captainstabs
dolt merge -m "some message" --no-ff remotes/captainstabs/marion
dolt merge -m "some other message" --no-ff remotes/captainstabs/NJ

The merges are working, but you'll see that the statistics for the number of rows changed are wrong for everything after the first merge.

@timsehn
Copy link
Sponsor Contributor

timsehn commented Aug 26, 2022

What did you get and what are you expecting?

@timsehn timsehn added the new format Issues related specifically to the __DOLT__ storage format label Aug 26, 2022
@timsehn
Copy link
Sponsor Contributor

timsehn commented Aug 26, 2022

Here's a simple repro:

test-merge-stats $ dolt init --new-format
Successfully initialized dolt data repository.
test-merge-stats $ dolt sql -q "create table t (c1 int primary key, c2 int)"
test-merge-stats $ dolt add .
test-merge-stats $ dolt commit -m "created table"
commit 7kdgvpoua4gack9f1q7to1q51pdgtm37 (HEAD -> main) 
Author: Tim Sehn <tim@dolthub.com>
Date:  Fri Aug 26 14:08:31 -0700 2022

        created table

test-merge-stats $ dolt branch b1
test-merge-stats $ dolt branch b2
test-merge-stats $ dolt checkout b1
Switched to branch 'b1'
test-merge-stats $ dolt sql -q "insert into t values (0,0), (1,1), (2,2)" 
Query OK, 3 rows affected
test-merge-stats $ dolt sql -q "insert into t values (3,0), (4,1), (5,2)" 
Query OK, 3 rows affected
test-merge-stats $ dolt commit -am "some data on b1"
commit f1hiarb44slku9n44t8i1p8qtbsulig0 (HEAD -> b1) 
Author: Tim Sehn <tim@dolthub.com>
Date:  Fri Aug 26 14:10:39 -0700 2022

        some data on b1

test-merge-stats $ dolt checkout b2
Switched to branch 'b2'
test-merge-stats $ dolt sql -q "insert into t values (6,0), (7,1), (8,2)" 
Query OK, 3 rows affected
test-merge-stats $ dolt commit -am "some data on b2"
commit 459jejvkp812fbqgf6rqvak4dsvjqco0 (HEAD -> b2) 
Author: Tim Sehn <tim@dolthub.com>
Date:  Fri Aug 26 14:11:01 -0700 2022

        some data on b2

test-merge-stats $ dolt checkout main
Switched to branch 'main'
test-merge-stats $ dolt merge b1;
Updating 7kdgvpoua4gack9f1q7to1q51pdgtm37..f1hiarb44slku9n44t8i1p8qtbsulig0
Fast-forward
test-merge-stats $ dolt merge b2;
Updating f1hiarb44slku9n44t8i1p8qtbsulig0..459jejvkp812fbqgf6rqvak4dsvjqco0
Waiting for command to finish.
t | 0 
1 tables changed, 0 rows added(+), 0 rows modified(*), 0 rows deleted(-)

Notice how it says 0 rows added, modified and deleted. This should say 3 rows added.

Seeing if this works in old format.

@timsehn
Copy link
Sponsor Contributor

timsehn commented Aug 26, 2022

Old format works:

text-merge-stats $ dolt init
Successfully initialized dolt data repository.
text-merge-stats $ dolt version
dolt version 0.40.29
database storage format: OLD ( __LD_1__ )
text-merge-stats $ dolt sql -q "create table t (c1 int primary key, c2 int)"
text-merge-stats $ dolt add .
text-merge-stats $ dolt commit -m "new table"
commit o1vdkv1smdmf147m5v7i4ff6cul51b8c (HEAD -> main) 
Author: Tim Sehn <tim@dolthub.com>
Date:  Fri Aug 26 14:14:52 -0700 2022

        new table

text-merge-stats $ dolt branch b1
text-merge-stats $ dolt branch b2
text-merge-stats $ dolt checkout b1
Switched to branch 'b1'
text-merge-stats $ dolt sql -q "insert into t values (0,0), (1,1), (2,2)" 
Query OK, 3 rows affected
text-merge-stats $ dolt commit -am "some data on b1"
commit pgdj3u8lpcdm1v83ogutrkae80v1ts97 (HEAD -> b1) 
Author: Tim Sehn <tim@dolthub.com>
Date:  Fri Aug 26 14:15:24 -0700 2022

        some data on b1

text-merge-stats $ dolt checkout b2
Switched to branch 'b2'
text-merge-stats $ dolt sql -q "insert into t values (3,0), (4,1), (5,2)" 
Query OK, 3 rows affected
text-merge-stats $ dolt commit -am "some data on b2"
commit bvnsg017lajfn4sfrhbmas3nfbe8skt7 (HEAD -> b2) 
Author: Tim Sehn <tim@dolthub.com>
Date:  Fri Aug 26 14:15:41 -0700 2022

        some data on b2

text-merge-stats $ dolt checkout main
Switched to branch 'main'
text-merge-stats $ dolt merge b1
Updating o1vdkv1smdmf147m5v7i4ff6cul51b8c..pgdj3u8lpcdm1v83ogutrkae80v1ts97
Fast-forward
text-merge-stats $ dolt merge b2
Updating pgdj3u8lpcdm1v83ogutrkae80v1ts97..bvnsg017lajfn4sfrhbmas3nfbe8skt7
Waiting for command to finish.
t | 3 +++
1 tables changed, 3 rows added(+), 0 rows modified(*), 0 rows deleted(-)

@druvv druvv self-assigned this Aug 26, 2022
@druvv
Copy link
Contributor

druvv commented Aug 26, 2022

Hey @alecstein, this isn't implemented in the new format. Fast-forwards will return the correct values but a three way merge will not. That's why you see statistics for some merges and not others.

@timsehn
Copy link
Sponsor Contributor

timsehn commented Sep 21, 2022

This was fixed in #4384.

@timsehn timsehn closed this as completed Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new format Issues related specifically to the __DOLT__ storage format
Projects
None yet
Development

No branches or pull requests

3 participants