diff --git a/go/libraries/doltcore/branch_control/access.go b/go/libraries/doltcore/branch_control/access.go index ab2be1feab7..feb14250c73 100644 --- a/go/libraries/doltcore/branch_control/access.go +++ b/go/libraries/doltcore/branch_control/access.go @@ -78,6 +78,7 @@ func (tbl *Access) Match(database string, branch string, user string, host strin for _, result := range results { if result.Length > length { perms = result.Permissions + length = result.Length } else if result.Length == length { perms |= result.Permissions } @@ -258,3 +259,16 @@ OuterLoop: } return AccessRow{}, false } + +// Consolidate reduces the permission set down to the most representative permission. For example, having both admin and +// write permissions are equivalent to only having the admin permission. Additionally, having no permissions is +// equivalent to only having the read permission. +func (perm Permissions) Consolidate() Permissions { + if perm&Permissions_Admin == Permissions_Admin { + return Permissions_Admin + } else if perm&Permissions_Write == Permissions_Write { + return Permissions_Write + } else { + return Permissions_Read + } +} diff --git a/go/libraries/doltcore/branch_control/namespace.go b/go/libraries/doltcore/branch_control/namespace.go index 6a11a52fa5c..d774755b11c 100644 --- a/go/libraries/doltcore/branch_control/namespace.go +++ b/go/libraries/doltcore/branch_control/namespace.go @@ -88,6 +88,7 @@ func (tbl *Namespace) CanCreate(database string, branch string, user string, hos matchedValue := tbl.Values[matched] // If we've found a longer match, then we reset the slice. We append to it in the following if statement. if len(matchedValue.Branch) > longest { + longest = len(matchedValue.Branch) filteredIndexes = filteredIndexes[:0] } if len(matchedValue.Branch) >= longest { diff --git a/go/libraries/doltcore/diff/table_deltas.go b/go/libraries/doltcore/diff/table_deltas.go index 93de7eff0e7..e91b7d3023a 100644 --- a/go/libraries/doltcore/diff/table_deltas.go +++ b/go/libraries/doltcore/diff/table_deltas.go @@ -404,6 +404,81 @@ func (td TableDelta) HasSchemaChanged(ctx context.Context) (bool, error) { return !fromSchemaHash.Equal(toSchemaHash), nil } +func (td TableDelta) HasChangesIgnoringColumnTags(ctx context.Context) (bool, error) { + + if td.FromTable == nil && td.ToTable == nil { + return true, nil + } + + if td.IsAdd() || td.IsDrop() { + return true, nil + } + + if td.IsRename() { + return true, nil + } + + if td.HasFKChanges() { + return true, nil + } + + fromRowDataHash, err := td.FromTable.GetRowDataHash(ctx) + if err != nil { + return false, err + } + + toRowDataHash, err := td.ToTable.GetRowDataHash(ctx) + if err != nil { + return false, err + } + + // Any change to the table data counts as a change + if !fromRowDataHash.Equal(toRowDataHash) { + return true, nil + } + + fromTableHash, err := td.FromTable.HashOf() + if err != nil { + return false, err + } + + toTableHash, err := td.FromTable.HashOf() + if err != nil { + return false, err + } + + // If the data hashes have changed, the table has obviously changed. + if !fromTableHash.Equal(toTableHash) { + return true, nil + } + + fromSchemaHash, err := td.FromTable.GetSchemaHash(ctx) + if err != nil { + return false, err + } + + toSchemaHash, err := td.ToTable.GetSchemaHash(ctx) + if err != nil { + return false, err + } + + // If neither data nor schema hashes have changed, the table is obviously the same. + if fromSchemaHash.Equal(toSchemaHash) { + return false, nil + } + + // The schema hash has changed but the data has remained the same. We must inspect the schema to determine + // whether the change is observable or if only column tags have changed. + + fromSchema, toSchema, err := td.GetSchemas(ctx) + if err != nil { + return false, err + } + + // SchemasAreEqual correctly accounts for tags + return !schema.SchemasAreEqual(fromSchema, toSchema), nil +} + func (td TableDelta) HasDataChanged(ctx context.Context) (bool, error) { // Database collation change is not a data change if td.FromTable == nil && td.ToTable == nil { @@ -445,6 +520,9 @@ func (td TableDelta) HasPrimaryKeySetChanged() bool { } func (td TableDelta) HasChanges() (bool, error) { + fromString := td.FromTable.DebugString(context.Background(), td.FromTable.NodeStore()) + toString := td.ToTable.DebugString(context.Background(), td.ToTable.NodeStore()) + _, _ = fromString, toString hashChanged, err := td.HasHashChanged() if err != nil { return false, err diff --git a/go/libraries/doltcore/sqle/dtables/branch_control_table.go b/go/libraries/doltcore/sqle/dtables/branch_control_table.go index ff7d4aa6e46..c27c8b9f94b 100644 --- a/go/libraries/doltcore/sqle/dtables/branch_control_table.go +++ b/go/libraries/doltcore/sqle/dtables/branch_control_table.go @@ -202,9 +202,10 @@ func (tbl BranchControlTable) Insert(ctx *sql.Context, row sql.Row) error { } } - // We check if we're inserting a subset of an already-existing row. If we are, we deny the insertion as the existing - // row will already match against ALL possible values for this row. - if ok, modPerms := tbl.Match(database, branch, user, host); ok { + // We check if we're inserting a subset of an already-existing row. We only consider this a subset if the + // permissions are as permissible as the existing ones, or are more restrictive (i.e. write is a "subset permission" + // of admin). If we are, we deny the insertion as the existing row will already match against ALL possible values for this row. + if ok, modPerms := tbl.Match(database, branch, user, host); ok && perms.Consolidate() >= modPerms.Consolidate() { permBits := uint64(modPerms) permStr, _ := accessSchema[4].Type.(sql.SetType).BitsToString(permBits) return sql.NewUniqueKeyErr( diff --git a/go/libraries/doltcore/sqle/dtables/status_table.go b/go/libraries/doltcore/sqle/dtables/status_table.go index 61add2cc494..e16fbacd5c1 100644 --- a/go/libraries/doltcore/sqle/dtables/status_table.go +++ b/go/libraries/doltcore/sqle/dtables/status_table.go @@ -132,6 +132,20 @@ func newStatusItr(ctx *sql.Context, st *StatusTable) (*StatusItr, error) { return nil, err } + // Some tables may differ only in column tags and/or recorded conflicts. + // We try to make such changes invisible to users and shouldn't display them for unstaged tables. + changedUnstagedTables := make([]diff.TableDelta, 0, len(unstagedTables)) + for _, unstagedTableDiff := range unstagedTables { + changed, err := unstagedTableDiff.HasChangesIgnoringColumnTags(ctx) + if err != nil { + return nil, err + } + if changed { + changedUnstagedTables = append(changedUnstagedTables, unstagedTableDiff) + } + } + unstagedTables = changedUnstagedTables + stagedSchemas, unstagedSchemas, err := diff.GetStagedUnstagedDatabaseSchemaDeltas(ctx, roots) if err != nil { return nil, err diff --git a/integration-tests/bats/branch-control.bats b/integration-tests/bats/branch-control.bats index 4c8afa859b3..c38a5d93a0b 100644 --- a/integration-tests/bats/branch-control.bats +++ b/integration-tests/bats/branch-control.bats @@ -13,7 +13,7 @@ teardown() { } setup_test_user() { - dolt sql -q "create user test" + dolt sql -q "create user test identified by ''" dolt sql -q "grant all on *.* to test" dolt sql -q "delete from dolt_branch_control where user='%'" } @@ -101,9 +101,9 @@ setup_test_user() { [ $status -ne 0 ] [[ $output =~ "does not have the correct permissions" ]] || false - dolt -u test sql -q "call dolt_checkout('test-branch'); create table t (c1 int)" + dolt -u test -p '' sql -q "call dolt_checkout('test-branch'); create table t (c1 int)" - dolt -u test sql -q "call dolt_checkout('test-branch'); call dolt_add('t'); call dolt_commit('-m', 'Testing commit');" + dolt -u test -p '' sql -q "call dolt_checkout('test-branch'); call dolt_add('t'); call dolt_commit('-m', 'Testing commit');" # I should also have branch permissions on branches I create dolt sql -q "call dolt_checkout('-b', 'test-branch-2'); create table t (c1 int)" @@ -117,7 +117,7 @@ setup_test_user() { @test "branch-control: test admin permissions" { setup_test_user - dolt sql -q "create user test2" + dolt sql -q "create user test2 identified by ''" dolt sql -q "grant all on *.* to test2" dolt sql -q "insert into dolt_branch_control values ('dolt-repo-$$', 'test-branch', 'test', '%', 'admin')" @@ -126,16 +126,16 @@ setup_test_user() { start_sql_server # Admin has no write permission to branch not an admin on - run dolt -u test sql -q "create table t (c1 int)" + run dolt -u test -p '' sql -q "create table t (c1 int)" [ $status -ne 0 ] [[ $output =~ "does not have the correct permissions" ]] || false # Admin can write - dolt -u test sql -q "call dolt_checkout('test-branch'); create table t (c1 int)" + dolt -u test -p '' sql -q "call dolt_checkout('test-branch'); create table t (c1 int)" # Admin can make other users - dolt -u test sql -q "insert into dolt_branch_control values ('dolt-repo-$$', 'test-branch', 'test2', '%', 'write')" - run dolt -u test sql --result-format csv -q "select * from dolt_branch_control" + dolt -u test -p '' sql -q "insert into dolt_branch_control values ('dolt-repo-$$', 'test-branch', 'test2', '%', 'write')" + run dolt -u test -p '' sql --result-format csv -q "select * from dolt_branch_control" [ $status -eq 0 ] [ ${lines[0]} = "database,branch,user,host,permissions" ] [ ${lines[1]} = "dolt-repo-$$,test-branch,test,%,admin" ] @@ -143,7 +143,7 @@ setup_test_user() { [ ${lines[3]} = "dolt-repo-$$,test-branch,test2,%,write" ] # test2 can see all branch permissions - run dolt -u test2 sql --result-format csv -q "select * from dolt_branch_control" + run dolt -u test2 -p '' sql --result-format csv -q "select * from dolt_branch_control" [ $status -eq 0 ] [ ${lines[0]} = "database,branch,user,host,permissions" ] [ ${lines[1]} = "dolt-repo-$$,test-branch,test,%,admin" ] @@ -151,18 +151,18 @@ setup_test_user() { [ ${lines[3]} = "dolt-repo-$$,test-branch,test2,%,write" ] # test2 now has write permissions on test-branch - dolt -u test2 sql -q "call dolt_checkout('test-branch'); insert into t values(0)" + dolt -u test2 -p '' sql -q "call dolt_checkout('test-branch'); insert into t values(0)" # Remove test2 permissions - dolt -u test sql -q "delete from dolt_branch_control where user='test2'" + dolt -u test -p '' sql -q "delete from dolt_branch_control where user='test2'" - run dolt -u test sql --result-format csv -q "select * from dolt_branch_control" + run dolt -u test -p '' sql --result-format csv -q "select * from dolt_branch_control" [ $status -eq 0 ] [ ${lines[0]} = "database,branch,user,host,permissions" ] [ ${lines[1]} = "dolt-repo-$$,test-branch,test,%,admin" ] # test2 cannot write to branch - run dolt -u test2 sql -q "call dolt_checkout('test-branch'); insert into t values(1)" + run dolt -u test2 -p '' sql -q "call dolt_checkout('test-branch'); insert into t values(1)" [ $status -ne 0 ] [[ $output =~ "does not have the correct permissions" ]] || false } @@ -174,9 +174,9 @@ setup_test_user() { start_sql_server - dolt -u test sql -q "call dolt_branch('test-branch')" + dolt -u test -p '' sql -q "call dolt_branch('test-branch')" - run dolt -u test sql --result-format csv -q "select * from dolt_branch_control" + run dolt -u test -p '' sql --result-format csv -q "select * from dolt_branch_control" [ $status -eq 0 ] [ ${lines[0]} = "database,branch,user,host,permissions" ] [ ${lines[1]} = "dolt-repo-$$,main,test,%,write" ] @@ -186,7 +186,7 @@ setup_test_user() { @test "branch-control: test branch namespace control" { setup_test_user - dolt sql -q "create user test2" + dolt sql -q "create user test2 identified by ''" dolt sql -q "grant all on *.* to test2" dolt sql -q "insert into dolt_branch_control values ('dolt-repo-$$', 'test- @@ -195,24 +195,24 @@ branch', 'test', '%', 'admin')" start_sql_server - run dolt -u test sql --result-format csv -q "select * from dolt_branch_namespace_control" + run dolt -u test -p '' sql --result-format csv -q "select * from dolt_branch_namespace_control" [ $status -eq 0 ] [ ${lines[0]} = "database,branch,user,host" ] [ ${lines[1]} = "dolt-repo-$$,test-%,test2,%" ] # test cannot create test-branch - run dolt -u test sql -q "call dolt_branch('test-branch')" + run dolt -u test -p '' sql -q "call dolt_branch('test-branch')" [ $status -ne 0 ] [[ $output =~ "cannot create a branch" ]] || false # test2 can create test-branch - dolt -u test2 sql -q "call dolt_branch('test-branch')" + dolt -u test2 -p '' sql -q "call dolt_branch('test-branch')" } @test "branch-control: test longest match in branch namespace control" { setup_test_user - dolt sql -q "create user test2" + dolt sql -q "create user test2 identified by ''" dolt sql -q "grant all on *.* to test2" dolt sql -q "insert into dolt_branch_namespace_control values ('dolt-repo-$$', 'test/%', 'test', '%')" @@ -220,21 +220,21 @@ branch', 'test', '%', 'admin')" start_sql_server - # test can create a branch in its namesapce but not in test2 - dolt -u test sql -q "call dolt_branch('test/branch1')" - run dolt -u test sql -q "call dolt_branch('test2/branch1')" + # test can create a branch in its namespace but not in test2 + dolt -u test -p '' sql -q "call dolt_branch('test/branch1')" + run dolt -u test -p '' sql -q "call dolt_branch('test2/branch1')" [ $status -ne 0 ] [[ $output =~ "cannot create a branch" ]] || false - dolt -u test2 sql -q "call dolt_branch('test2/branch1')" - run dolt -u test2 sql -q "call dolt_branch('test/branch1')" + dolt -u test2 -p '' sql -q "call dolt_branch('test2/branch1')" + run dolt -u test2 -p '' sql -q "call dolt_branch('test/branch1')" [ $status -ne 0 ] [[ $output =~ "cannot create a branch" ]] || false } @test "branch-control: test longest match in branch access control" { setup_test_user - dolt sql -q "create user admin" + dolt sql -q "create user admin identified by ''" dolt sql -q "grant all on *.* to admin" dolt sql -q "insert into dolt_branch_control values ('%', '%', 'admin', '%', 'admin')" @@ -244,13 +244,13 @@ branch', 'test', '%', 'admin')" start_sql_server - run dolt -u test sql -q "call dolt_checkout('test-branch'); create table t (c1 int)" + run dolt -u test -p '' sql -q "call dolt_checkout('test-branch'); create table t (c1 int)" [ $status -ne 0 ] [[ $output =~ "does not have the correct permissions" ]] || false - dolt -u admin sql -q "delete from dolt_branch_control where branch = 'test-branch'" + dolt -u admin -p '' sql -q "delete from dolt_branch_control where branch = 'test-branch'" - run dolt -u test sql -q "call dolt_checkout('test-branch'); create table t (c1 int)" + run dolt -u test -p '' sql -q "call dolt_checkout('test-branch'); create table t (c1 int)" [ $status -eq 0 ] [[ ! $output =~ "does not have the correct permissions" ]] || false } @@ -295,3 +295,64 @@ SQL [ $status -eq 0 ] [[ $output =~ "0 rows affected" ]] || false } + +@test "branch-control: Issue #8622 ttask" { + # https://github.com/dolthub/dolt/issues/8622 + dolt sql < $logFile 2>&1 & + if [ "$IS_WINDOWS" == true ]; then + dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" > $logFile 2>&1 & + else + dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" --socket "dolt.$PORT.sock" > $logFile 2>&1 & + fi else - dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" --socket "dolt.$PORT.sock" & + if [ "$IS_WINDOWS" == true ]; then + dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" & + else + dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" --socket "dolt.$PORT.sock" & + fi fi echo db:$DEFAULT_DB logFile:$logFile PORT:$PORT CWD:$PWD SERVER_PID=$! @@ -53,7 +62,11 @@ start_sql_server() { start_sql_server_with_args() { DEFAULT_DB="" PORT=$( definePORT ) - dolt sql-server "$@" --port=$PORT --socket "dolt.$PORT.sock" & + if [ "$IS_WINDOWS" == true ]; then + dolt sql-server "$@" --port=$PORT & + else + dolt sql-server "$@" --port=$PORT --socket "dolt.$PORT.sock" & + fi SERVER_PID=$! wait_for_connection $PORT 8500 } @@ -76,7 +89,11 @@ behavior: autocommit: false " > .cliconfig.yaml cat "$2" >> .cliconfig.yaml - dolt sql-server --config .cliconfig.yaml --socket "dolt.$PORT.sock" & + if [ "$IS_WINDOWS" == true ]; then + dolt sql-server --config .cliconfig.yaml & + else + dolt sql-server --config .cliconfig.yaml --socket "dolt.$PORT.sock" & + fi SERVER_PID=$! wait_for_connection $PORT 8500 } @@ -98,7 +115,11 @@ listener: behavior: autocommit: false " > .cliconfig.yaml - dolt sql-server --config .cliconfig.yaml --socket "dolt.$PORT.sock" & + if [ "$IS_WINDOWS" == true ]; then + dolt sql-server --config .cliconfig.yaml & + else + dolt sql-server --config .cliconfig.yaml --socket "dolt.$PORT.sock" & + fi SERVER_PID=$! wait_for_connection $PORT 8500 } @@ -106,7 +127,11 @@ behavior: start_multi_db_server() { DEFAULT_DB="$1" PORT=$( definePORT ) - dolt sql-server --host 0.0.0.0 --port=$PORT --user dolt --data-dir ./ --socket "dolt.$PORT.sock" & + if [ "$IS_WINDOWS" == true ]; then + dolt sql-server --host 0.0.0.0 --port=$PORT --user dolt --data-dir ./ & + else + dolt sql-server --host 0.0.0.0 --port=$PORT --user dolt --data-dir ./ --socket "dolt.$PORT.sock" & + fi SERVER_PID=$! wait_for_connection $PORT 8500 } diff --git a/integration-tests/bats/sql-status.bats b/integration-tests/bats/sql-status.bats index fdbf58f418f..9a8823c208d 100644 --- a/integration-tests/bats/sql-status.bats +++ b/integration-tests/bats/sql-status.bats @@ -150,5 +150,32 @@ teardown() { run dolt sql -r csv -q "select * from dolt_status ORDER BY status" [ "$status" -eq 0 ] [[ "$output" =~ 'dolt_docs,false,conflict' ]] || false - [[ "$output" =~ 'dolt_docs,false,modified' ]] || false -} \ No newline at end of file +} + +@test "sql-status: tables with no observable changes don't show in status but can be staged" { +dolt sql <