mirror of
https://github.com/dolthub/dolt.git
synced 2026-02-25 10:19:24 -06:00
fix(sqle): address PR feedback for dolt_status_ignored; fix & add tests
Address PR review feedback for dolt_status_ignored implementation with refactors to error handling, adapter pattern, test coverage, and copyright year. Changes: - Fix copyright year from 2020 to 2025 in status_ignored_table.go - Add adapter pattern to NewStatusIgnoredTable; matches StatusTable implementation. - Add NewStatusIgnoredTableWithNoAdapter for default behavior - Fix error handling in checkIfIgnored to return errors instead of silently returning byte(0) - Propagate ignore pattern errors up the call stack Test coverage additions: - Empty dolt_ignore table test verifying all tables show ignored=0 - Conflicting patterns test with wildcard (test_*=true) vs specific override (test_special=false) - Update ls.bats to expect 27 system tables instead of 26 Error handling refactored to now matches patterns in status.go, commit .go, and diff.go where IsTableNameIgnored errors are properly propagated up rather than swallowed. Refs: #5862
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
// Copyright 2020 Dolthub, Inc.
|
||||
// Copyright 2025 Dolthub, Inc.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
@@ -24,6 +24,7 @@ import (
|
||||
"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
|
||||
"github.com/dolthub/dolt/go/libraries/doltcore/env"
|
||||
"github.com/dolthub/dolt/go/libraries/doltcore/schema"
|
||||
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/adapters"
|
||||
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/index"
|
||||
)
|
||||
|
||||
@@ -82,8 +83,19 @@ func (st StatusIgnoredTable) PartitionRows(context *sql.Context, _ sql.Partition
|
||||
return newStatusIgnoredItr(context, &st)
|
||||
}
|
||||
|
||||
// NewStatusIgnoredTable creates a new StatusIgnoredTable.
|
||||
func NewStatusIgnoredTable(_ *sql.Context, tableName string, ddb *doltdb.DoltDB, ws *doltdb.WorkingSet, rp env.RootsProvider[*sql.Context]) sql.Table {
|
||||
// NewStatusIgnoredTable creates a new StatusIgnoredTable using either an integrators' [adapters.TableAdapter] or the
|
||||
// NewStatusIgnoredTableWithNoAdapter constructor (the default implementation provided by Dolt).
|
||||
func NewStatusIgnoredTable(ctx *sql.Context, tableName string, ddb *doltdb.DoltDB, ws *doltdb.WorkingSet, rp env.RootsProvider[*sql.Context]) sql.Table {
|
||||
adapter, ok := adapters.DoltTableAdapterRegistry.GetAdapter(tableName)
|
||||
if ok {
|
||||
return adapter.NewTable(ctx, tableName, ddb, ws, rp)
|
||||
}
|
||||
|
||||
return NewStatusIgnoredTableWithNoAdapter(ctx, tableName, ddb, ws, rp)
|
||||
}
|
||||
|
||||
// NewStatusIgnoredTableWithNoAdapter returns a new StatusIgnoredTable.
|
||||
func NewStatusIgnoredTableWithNoAdapter(_ *sql.Context, tableName string, ddb *doltdb.DoltDB, ws *doltdb.WorkingSet, rp env.RootsProvider[*sql.Context]) sql.Table {
|
||||
return &StatusIgnoredTable{
|
||||
tableName: tableName,
|
||||
ddb: ddb,
|
||||
@@ -131,7 +143,11 @@ func newStatusIgnoredItr(ctx *sql.Context, st *StatusIgnoredTable) (*StatusIgnor
|
||||
ignored := byte(0)
|
||||
// Only check ignore patterns for unstaged tables
|
||||
if row.isStaged == byte(0) && unstagedTableNames[row.tableName] {
|
||||
ignored = checkIfIgnored(row.tableName, ignorePatterns)
|
||||
var err error
|
||||
ignored, err = checkIfIgnored(row.tableName, ignorePatterns)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
rows[i] = statusIgnoredTableRow{
|
||||
tableName: row.tableName,
|
||||
@@ -168,16 +184,16 @@ func buildUnstagedTableNameSet(unstagedTables []diff.TableDelta) map[string]bool
|
||||
}
|
||||
|
||||
// checkIfIgnored returns 1 if the table name matches an ignore pattern, 0 otherwise.
|
||||
func checkIfIgnored(tableName string, ignorePatterns doltdb.IgnorePatterns) byte {
|
||||
func checkIfIgnored(tableName string, ignorePatterns doltdb.IgnorePatterns) (byte, error) {
|
||||
tblNameObj := doltdb.TableName{Name: tableName}
|
||||
result, err := ignorePatterns.IsTableNameIgnored(tblNameObj)
|
||||
if err != nil {
|
||||
return byte(0)
|
||||
return byte(0), err
|
||||
}
|
||||
if result == doltdb.Ignore {
|
||||
return byte(1)
|
||||
return byte(1), nil
|
||||
}
|
||||
return byte(0)
|
||||
return byte(0), nil
|
||||
}
|
||||
|
||||
// Next retrieves the next row. It will return io.EOF if it's the last row.
|
||||
|
||||
@@ -759,6 +759,157 @@ var DoltScripts = []queries.ScriptTest{
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "dolt_status_ignored basic tests",
|
||||
SetUpScript: []string{
|
||||
"CREATE TABLE t (pk int primary key);",
|
||||
"INSERT INTO dolt_ignore VALUES ('ignored_*', true);",
|
||||
"CREATE TABLE ignored_test (pk int primary key);",
|
||||
},
|
||||
Assertions: []queries.ScriptTestAssertion{
|
||||
{
|
||||
// Verify schema has 4 columns
|
||||
Query: "DESCRIBE dolt_status_ignored;",
|
||||
Expected: []sql.Row{
|
||||
{"table_name", "text", "NO", "PRI", nil, ""},
|
||||
{"staged", "tinyint(1)", "NO", "PRI", nil, ""},
|
||||
{"status", "text", "NO", "PRI", nil, ""},
|
||||
{"ignored", "tinyint(1)", "NO", "", nil, ""},
|
||||
},
|
||||
},
|
||||
{
|
||||
// Non-ignored unstaged table has ignored=0
|
||||
Query: "SELECT table_name, staged, status, ignored FROM dolt_status_ignored WHERE table_name = 't';",
|
||||
Expected: []sql.Row{{"t", byte(0), "new table", byte(0)}},
|
||||
},
|
||||
{
|
||||
// Ignored unstaged table has ignored=1
|
||||
Query: "SELECT table_name, staged, status, ignored FROM dolt_status_ignored WHERE table_name = 'ignored_test';",
|
||||
Expected: []sql.Row{{"ignored_test", byte(0), "new table", byte(1)}},
|
||||
},
|
||||
{
|
||||
// dolt_ignore table itself shows as not ignored
|
||||
Query: "SELECT table_name, ignored FROM dolt_status_ignored WHERE table_name = 'dolt_ignore';",
|
||||
Expected: []sql.Row{{"dolt_ignore", byte(0)}},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "dolt_status_ignored staged tables always have ignored=0",
|
||||
SetUpScript: []string{
|
||||
// Create and stage table BEFORE adding ignore pattern
|
||||
"CREATE TABLE staged_test (pk int primary key);",
|
||||
"CALL DOLT_ADD('staged_test');",
|
||||
// Now add pattern that matches the already-staged table name
|
||||
"INSERT INTO dolt_ignore VALUES ('staged_*', true);",
|
||||
},
|
||||
Assertions: []queries.ScriptTestAssertion{
|
||||
{
|
||||
// Staged table has ignored=0 even if name matches ignore pattern
|
||||
Query: "SELECT table_name, staged, ignored FROM dolt_status_ignored WHERE table_name = 'staged_test';",
|
||||
Expected: []sql.Row{{"staged_test", byte(1), byte(0)}},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "dolt_status_ignored with AS OF and branch queries",
|
||||
SetUpScript: []string{
|
||||
"CALL DOLT_COMMIT('--allow-empty', '-m', 'empty commit');",
|
||||
"SET @commit1 = HASHOF('HEAD');",
|
||||
"CALL DOLT_TAG('tag1');",
|
||||
"CALL DOLT_CHECKOUT('-b', 'branch1');",
|
||||
"CREATE TABLE abc (pk int);",
|
||||
"CALL DOLT_ADD('abc');",
|
||||
"CALL DOLT_CHECKOUT('main');",
|
||||
"CREATE TABLE t (pk int primary key);",
|
||||
},
|
||||
Assertions: []queries.ScriptTestAssertion{
|
||||
{
|
||||
Query: "SELECT table_name, staged, status, ignored FROM dolt_status_ignored;",
|
||||
Expected: []sql.Row{{"t", byte(0), "new table", byte(0)}},
|
||||
},
|
||||
{
|
||||
Query: "SELECT * FROM dolt_status_ignored AS OF 'tag1';",
|
||||
Expected: []sql.Row{},
|
||||
},
|
||||
{
|
||||
Query: "SELECT * FROM dolt_status_ignored AS OF @commit1;",
|
||||
Expected: []sql.Row{},
|
||||
},
|
||||
{
|
||||
Query: "SELECT table_name, staged, status, ignored FROM dolt_status_ignored AS OF 'branch1';",
|
||||
Expected: []sql.Row{{"abc", byte(1), "new table", byte(0)}},
|
||||
},
|
||||
{
|
||||
Query: "SELECT table_name, staged, status, ignored FROM `mydb/branch1`.dolt_status_ignored;",
|
||||
Expected: []sql.Row{{"abc", byte(1), "new table", byte(0)}},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "dolt_status_ignored with multiple ignore patterns",
|
||||
SetUpScript: []string{
|
||||
"INSERT INTO dolt_ignore VALUES ('temp_*', true);",
|
||||
"INSERT INTO dolt_ignore VALUES ('*_backup', true);",
|
||||
"CREATE TABLE temp_data (pk int primary key);",
|
||||
"CREATE TABLE users_backup (pk int primary key);",
|
||||
"CREATE TABLE normal_table (pk int primary key);",
|
||||
},
|
||||
Assertions: []queries.ScriptTestAssertion{
|
||||
{
|
||||
Query: "SELECT table_name, ignored FROM dolt_status_ignored WHERE table_name = 'temp_data';",
|
||||
Expected: []sql.Row{{"temp_data", byte(1)}},
|
||||
},
|
||||
{
|
||||
Query: "SELECT table_name, ignored FROM dolt_status_ignored WHERE table_name = 'users_backup';",
|
||||
Expected: []sql.Row{{"users_backup", byte(1)}},
|
||||
},
|
||||
{
|
||||
Query: "SELECT table_name, ignored FROM dolt_status_ignored WHERE table_name = 'normal_table';",
|
||||
Expected: []sql.Row{{"normal_table", byte(0)}},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "dolt_status_ignored with empty dolt_ignore",
|
||||
SetUpScript: []string{
|
||||
"CREATE TABLE t1 (pk int primary key);",
|
||||
"CREATE TABLE t2 (pk int primary key);",
|
||||
},
|
||||
Assertions: []queries.ScriptTestAssertion{
|
||||
{
|
||||
// With no ignore patterns, all tables should have ignored=0
|
||||
Query: "SELECT table_name, ignored FROM dolt_status_ignored ORDER BY table_name;",
|
||||
Expected: []sql.Row{
|
||||
{"t1", byte(0)},
|
||||
{"t2", byte(0)},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "dolt_status_ignored with conflicting patterns",
|
||||
SetUpScript: []string{
|
||||
// First pattern ignores tables starting with "test_"
|
||||
"INSERT INTO dolt_ignore VALUES ('test_*', true);",
|
||||
// Second pattern un-ignores specific table
|
||||
"INSERT INTO dolt_ignore VALUES ('test_special', false);",
|
||||
"CREATE TABLE test_normal (pk int primary key);",
|
||||
"CREATE TABLE test_special (pk int primary key);",
|
||||
},
|
||||
Assertions: []queries.ScriptTestAssertion{
|
||||
{
|
||||
// test_normal matches ignore pattern
|
||||
Query: "SELECT table_name, ignored FROM dolt_status_ignored WHERE table_name = 'test_normal';",
|
||||
Expected: []sql.Row{{"test_normal", byte(1)}},
|
||||
},
|
||||
{
|
||||
// test_special is explicitly not ignored (false overrides wildcard)
|
||||
Query: "SELECT table_name, ignored FROM dolt_status_ignored WHERE table_name = 'test_special';",
|
||||
Expected: []sql.Row{{"test_special", byte(0)}},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "dolt_hashof_table tests",
|
||||
SetUpScript: []string{
|
||||
|
||||
@@ -60,9 +60,10 @@ teardown() {
|
||||
@test "ls: --system shows system tables" {
|
||||
run dolt ls --system
|
||||
[ "$status" -eq 0 ]
|
||||
[ "${#lines[@]}" -eq 26 ]
|
||||
[ "${#lines[@]}" -eq 27 ]
|
||||
[[ "$output" =~ "System tables:" ]] || false
|
||||
[[ "$output" =~ "dolt_status" ]] || false
|
||||
[[ "$output" =~ "dolt_status_ignored" ]] || false
|
||||
[[ "$output" =~ "dolt_commits" ]] || false
|
||||
[[ "$output" =~ "dolt_commit_ancestors" ]] || false
|
||||
[[ "$output" =~ "dolt_constraint_violations" ]] || false
|
||||
|
||||
Reference in New Issue
Block a user