From 54a40eed532c325956c1891d67d7b2ec248fda91 Mon Sep 17 00:00:00 2001 From: Tim Sehn Date: Tue, 17 Jun 2025 11:58:07 -0700 Subject: [PATCH] Another pas. This time beter i think --- go/libraries/doltcore/sqle/database.go | 47 +++++++++++++++++-- .../doltcore/sqle/dolt_schemas_diff_table.go | 30 ++++++++++-- .../dolt_schemas_history_diff_test.go | 30 ++++++++---- integration-tests/bats/system-tables.bats | 45 +++++++++++------- 4 files changed, 116 insertions(+), 36 deletions(-) diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go index 970739d15f..d62a9b3e92 100644 --- a/go/libraries/doltcore/sqle/database.go +++ b/go/libraries/doltcore/sqle/database.go @@ -334,6 +334,14 @@ func (db Database) getTableInsensitive(ctx *sql.Context, head *doltdb.Commit, ds switch { case lwrName == doltdb.DoltDiffTablePrefix+doltdb.SchemasTableName: // Special handling for dolt_diff_dolt_schemas + // For schema diff tables, we want to show all schema objects going from the initial commit + // to their current defined state, similar to how regular diff tables work + + // Get the initial commit (first commit in the database) + var initialCommit *doltdb.Commit + var initialCommitHash string + + // Get the initial commit by walking back to the beginning if head == nil { var err error head, err = ds.GetHeadCommit(ctx, db.RevisionQualifiedName()) @@ -341,15 +349,44 @@ func (db Database) getTableInsensitive(ctx *sql.Context, head *doltdb.Commit, ds return nil, false, err } } - - // Get working root and head root for diff - headRoot, err := head.GetRootValue(ctx) + + // Walk back to find the initial commit + initialCommit = head + for { + parents, err := initialCommit.ParentHashes(ctx) + if err != nil { + return nil, false, err + } + if len(parents) == 0 { + // This is the initial commit + break + } + // Get the first parent (main line) + parentOptCommit, err := db.ddb.ReadCommit(ctx, parents[0]) + if err != nil { + return nil, false, err + } + parentCommit, ok := parentOptCommit.ToCommit() + if !ok { + return nil, false, fmt.Errorf("expected commit, got ghost commit") + } + initialCommit = parentCommit + } + + // Get the hash and root of the initial commit + initialHash, err := initialCommit.HashOf() + if err != nil { + return nil, false, err + } + initialCommitHash = initialHash.String() + + initialRoot, err := initialCommit.GetRootValue(ctx) if err != nil { return nil, false, err } - // For diff tables, we compare HEAD vs working to show changes from HEAD to working - return NewDoltSchemasDiffTable(ctx, db.ddb, headRoot, root, "HEAD", "WORKING", db), true, nil + // Compare initial commit state vs current working state to show all changes since inception + return NewDoltSchemasDiffTable(ctx, db.ddb, initialRoot, root, initialCommitHash, "WORKING", db), true, nil case strings.HasPrefix(lwrName, doltdb.DoltDiffTablePrefix): if head == nil { diff --git a/go/libraries/doltcore/sqle/dolt_schemas_diff_table.go b/go/libraries/doltcore/sqle/dolt_schemas_diff_table.go index 797a744d30..4c9c84c30a 100644 --- a/go/libraries/doltcore/sqle/dolt_schemas_diff_table.go +++ b/go/libraries/doltcore/sqle/dolt_schemas_diff_table.go @@ -25,6 +25,7 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" + "github.com/dolthub/dolt/go/store/hash" ) const ( @@ -347,7 +348,7 @@ func (dsdri *doltSchemasDiffRowIter) createDiffRow(toRow, fromRow sql.Row, diffT if toRow != nil && len(toRow) >= 5 { copy(row[0:5], toRow[0:5]) // to_type, to_name, to_fragment, to_extra, to_sql_mode row[5] = dsdri.toRef // to_commit - row[6] = nil // to_commit_date (we'll set this later if needed) + row[6] = nil // to_commit_date (null for WORKING state) } else { // All to_* columns are null for removed rows for i := 0; i < 7; i++ { @@ -359,12 +360,14 @@ func (dsdri *doltSchemasDiffRowIter) createDiffRow(toRow, fromRow sql.Row, diffT if fromRow != nil && len(fromRow) >= 5 { copy(row[7:12], fromRow[0:5]) // from_type, from_name, from_fragment, from_extra, from_sql_mode row[12] = dsdri.fromRef // from_commit - row[13] = nil // from_commit_date (we'll set this later if needed) + row[13] = dsdri.getFromCommitDate() // from_commit_date from actual commit } else { - // All from_* columns are null for added rows - for i := 7; i < 14; i++ { + // from_* schema columns are null for added rows, but commit info should be populated + for i := 7; i < 12; i++ { row[i] = nil } + row[12] = dsdri.fromRef // from_commit should always be populated (actual commit hash) + row[13] = dsdri.getFromCommitDate() // from_commit_date from actual commit } // Diff type column (index 14) @@ -373,6 +376,25 @@ func (dsdri *doltSchemasDiffRowIter) createDiffRow(toRow, fromRow sql.Row, diffT return row } +// getFromCommitDate gets the commit date for the from_commit +func (dsdri *doltSchemasDiffRowIter) getFromCommitDate() interface{} { + // If fromRef is a commit hash, try to get its commit date + if dsdri.fromRef != "WORKING" && dsdri.fromRef != "EMPTY" && len(dsdri.fromRef) > 0 { + // Try to parse fromRef as a commit hash and get its date + if hash.IsValid(dsdri.fromRef) { + commitHash := hash.Parse(dsdri.fromRef) + if optCommit, err := dsdri.db.GetDoltDB().ReadCommit(dsdri.ctx, commitHash); err == nil { + if commit, ok := optCommit.ToCommit(); ok { + if meta, err := commit.GetCommitMeta(dsdri.ctx); err == nil { + return meta.Time() + } + } + } + } + } + return nil +} + // rowsEqual compares two SQL rows for equality func rowsEqual(row1, row2 sql.Row) bool { if len(row1) != len(row2) { diff --git a/go/libraries/doltcore/sqle/integration_test/dolt_schemas_history_diff_test.go b/go/libraries/doltcore/sqle/integration_test/dolt_schemas_history_diff_test.go index d0f90ce85b..ee5fce46a1 100644 --- a/go/libraries/doltcore/sqle/integration_test/dolt_schemas_history_diff_test.go +++ b/go/libraries/doltcore/sqle/integration_test/dolt_schemas_history_diff_test.go @@ -196,8 +196,7 @@ func doltSchemasDiffTableTests() []doltSchemasTableTest { rows: []sql.Row{ {"event", "new_event", "added"}, {"view", "new_view", "added"}, - {"view", "original_view", "modified"}, - {nil, nil, "removed"}, // removed items have NULL to_ values + {"view", "original_view", "added"}, // all current objects show as "added" from empty state }, }, { @@ -206,20 +205,21 @@ func doltSchemasDiffTableTests() []doltSchemasTableTest { rows: []sql.Row{ {"event", "new_event"}, {"view", "new_view"}, + {"view", "original_view"}, // original_view also shows as "added" from empty state }, }, { name: "filter for modified schemas only", query: "SELECT to_type, to_name FROM dolt_diff_dolt_schemas WHERE diff_type = 'modified' ORDER BY to_type, to_name", rows: []sql.Row{ - {"view", "original_view"}, + // No modified entries with EMPTY vs WORKING comparison }, }, { name: "filter for removed schemas only", query: "SELECT from_type, from_name FROM dolt_diff_dolt_schemas WHERE diff_type = 'removed' ORDER BY from_type, from_name", rows: []sql.Row{ - {"trigger", "original_trigger"}, + // No removed entries with EMPTY vs WORKING comparison }, }, { @@ -227,23 +227,35 @@ func doltSchemasDiffTableTests() []doltSchemasTableTest { query: "SELECT COALESCE(to_name, from_name) as name, diff_type FROM dolt_diff_dolt_schemas WHERE COALESCE(to_type, from_type) = 'view' ORDER BY name", rows: []sql.Row{ {"new_view", "added"}, - {"original_view", "modified"}, + {"original_view", "added"}, // original_view shows as "added" from empty state }, }, { name: "count changes by type", query: "SELECT diff_type, COUNT(*) as count FROM dolt_diff_dolt_schemas GROUP BY diff_type ORDER BY diff_type", rows: []sql.Row{ - {"added", int64(2)}, - {"modified", int64(1)}, - {"removed", int64(1)}, + {"added", int64(3)}, // All 3 current objects show as "added" from empty state }, }, { name: "check all columns exist", query: "SELECT COUNT(*) FROM dolt_diff_dolt_schemas WHERE (to_type IS NOT NULL OR from_type IS NOT NULL) AND (to_name IS NOT NULL OR from_name IS NOT NULL) AND diff_type IS NOT NULL", rows: []sql.Row{ - {int64(4)}, // Total number of changes + {int64(3)}, // Total number of changes (all current objects as "added") + }, + }, + { + name: "verify commit columns are populated", + query: "SELECT COUNT(*) FROM dolt_diff_dolt_schemas WHERE to_commit IS NOT NULL AND from_commit IS NOT NULL", + rows: []sql.Row{ + {int64(3)}, // All rows should have both commit fields populated + }, + }, + { + name: "verify to_commit is WORKING and from_commit is a valid hash", + query: "SELECT DISTINCT to_commit, LENGTH(from_commit) as from_commit_len FROM dolt_diff_dolt_schemas", + rows: []sql.Row{ + {"WORKING", int32(32)}, // to_commit should be WORKING, from_commit should be 32-char hash }, }, } diff --git a/integration-tests/bats/system-tables.bats b/integration-tests/bats/system-tables.bats index 45837ae894..d807d1d6b7 100644 --- a/integration-tests/bats/system-tables.bats +++ b/integration-tests/bats/system-tables.bats @@ -550,8 +550,9 @@ SQL run dolt sql -q "select * from dolt_diff_dolt_schemas" [ "$status" -eq 0 ] - # After committing, there should be no diff between HEAD and WORKING - # This tests that our fix for the empty diff bug is working + # Should show the view and trigger as "added" from NULL (empty state) + [[ "$output" =~ "original_view" ]] || false + [[ "$output" =~ "original_trigger" ]] || false # Make changes for diff (working directory changes) dolt sql -q "DROP VIEW original_view" @@ -579,43 +580,51 @@ SQL [[ "$output" =~ "from_commit_date,datetime(6)" ]] || false [[ "$output" =~ "diff_type,varchar(1023)" ]] || false - # Test actual diff functionality - should show 4 changes (2 added, 1 modified, 1 removed) + # Test actual diff functionality - should show 3 changes (all current objects as "added" from empty state) + # With EMPTY vs WORKING comparison: original_view (modified), new_view (added), new_event (added) + # The removed trigger doesn't appear because it doesn't exist in WORKING run dolt sql -q 'SELECT COUNT(*) FROM dolt_diff_dolt_schemas' [ "$status" -eq 0 ] - [[ "$output" =~ "4" ]] || false + [[ "$output" =~ "3" ]] || false - # Test added schemas (new_view and new_event) + # Test that all schemas show as "added" from empty state run dolt sql -q 'SELECT COUNT(*) FROM dolt_diff_dolt_schemas WHERE diff_type = "added"' [ "$status" -eq 0 ] - [[ "$output" =~ "2" ]] || false + [[ "$output" =~ "3" ]] || false - # Test removed schemas (original_trigger) + # With EMPTY vs WORKING, there should be no "removed" or "modified" entries run dolt sql -q 'SELECT COUNT(*) FROM dolt_diff_dolt_schemas WHERE diff_type = "removed"' [ "$status" -eq 0 ] - [[ "$output" =~ "1" ]] || false + [[ "$output" =~ "0" ]] || false - # Test modified schemas (original_view that was dropped and recreated with different definition) run dolt sql -q 'SELECT COUNT(*) FROM dolt_diff_dolt_schemas WHERE diff_type = "modified"' [ "$status" -eq 0 ] - [[ "$output" =~ "1" ]] || false + [[ "$output" =~ "0" ]] || false # Test that we can identify specific added objects run dolt sql -q 'SELECT to_name FROM dolt_diff_dolt_schemas WHERE diff_type = "added" ORDER BY to_name' [ "$status" -eq 0 ] [[ "$output" =~ "new_event" ]] || false [[ "$output" =~ "new_view" ]] || false + [[ "$output" =~ "original_view" ]] || false - # Test that we can identify specific removed objects - run dolt sql -q 'SELECT from_name FROM dolt_diff_dolt_schemas WHERE diff_type = "removed"' - [ "$status" -eq 0 ] - [[ "$output" =~ "original_trigger" ]] || false - - # Test that diff_type values are correct + # Test that diff_type values are correct (only "added" with EMPTY vs WORKING) run dolt sql -q 'SELECT DISTINCT diff_type FROM dolt_diff_dolt_schemas ORDER BY diff_type' [ "$status" -eq 0 ] [[ "$output" =~ "added" ]] || false - [[ "$output" =~ "modified" ]] || false - [[ "$output" =~ "removed" ]] || false + + # Test that from_commit is always populated (should be the initial commit hash) + run dolt sql -q 'SELECT COUNT(*) FROM dolt_diff_dolt_schemas WHERE from_commit IS NOT NULL' + [ "$status" -eq 0 ] + [[ "$output" =~ "3" ]] || false + + # Test that from_commit is a valid commit hash (not "EMPTY" or "WORKING") + run dolt sql -q 'SELECT DISTINCT from_commit FROM dolt_diff_dolt_schemas' + [ "$status" -eq 0 ] + [[ ! "$output" =~ "EMPTY" ]] || false + [[ ! "$output" =~ "WORKING" ]] || false + # Should be a 32-character hash (using Dolt's character set) + [[ "$output" =~ [a-z0-9]{32} ]] || false } @test "system-tables: query dolt_history_ system table" {