dolthub / dolt

Dolt – Git for Data
Apache License 2.0
17.89k stars 507 forks source link

[no-release-notes] Fix issue where `dolt diff` fails to display changes to database schema if database name begins or ends with A, B, D, E, S, T, or _ (case sensitive) #8142

Closed nicktobey closed 3 months ago

nicktobey commented 3 months ago

This was discovered by running golangci-lint, which returned the following check failure:

cmd/dolt/commands/diff.go:913:38: SA1024: cutset contains duplicate characters (staticcheck)
        tableName = strings.Trim(tableName, diff.DBPrefix)

Reproduction example:

> # correct behavior
> mkdir doltRepo
> cd doltRepo
> dolt init
> dolt sql -q "ALTER DATABASE doltRepo COLLATE latin1_swedish_ci;"
> dolt diff
diff --dolt a/__DATABASE__doltRepo b/__DATABASE__doltRepo
--- a/__DATABASE__doltRepo
+++ b/__DATABASE__doltRepo
+CREATE DATABASE `doltRepo` /*!40100 DEFAULT CHARACTER SET latin1 COLLATE latin1_swedish_ci */;

> # incorrect behavior
> cd ..
> mv doltRepo DoltRepo
> cd DoltRepo
> dolt diff
diff --dolt a/__DATABASE__DoltRepo b/__DATABASE__DoltRepo
--- a/__DATABASE__DoltRepo
+++ b/__DATABASE__DoltRepo
Primary key sets differ between revisions for table '__DATABASE__DoltRepo', skipping data diff

The culprit is this line in diff.go:

tableName = strings.Trim(tableName, diff.DBPrefix)

strings.Trim does not remove a prefix. Rather, it removes every character from the start and end of the input string that matches a character in the second parameter. In this case, the second parameter is the constant string __DATABASE__, leading to all of these characters getting stripped from the table name.

We actually want to call strings.TrimPrefix instead.

coffeegoddd commented 3 months ago

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000
version result total
4c330aa ok 5937457
version total_tests
4c330aa 5937457
correctness_percentage
100.0
coffeegoddd commented 3 months ago

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000
version result total
e658448 ok 5937457
version total_tests
e658448 5937457
correctness_percentage
100.0
coffeegoddd commented 3 months ago

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000
version result total
fde045c ok 5937457
version total_tests
fde045c 5937457
correctness_percentage
100.0
coffeegoddd commented 3 months ago

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000
version result total
395d903 ok 5937457
version total_tests
395d903 5937457
correctness_percentage
100.0