From 6bffd93bf788b0a5e9f12f4f791db134ed6cd45d Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 18 Jun 2025 12:37:03 -0700 Subject: [PATCH] Bug fix, small refactorings --- go/libraries/doltcore/sqle/database.go | 20 ++++++++++++------- .../doltcore/sqle/dsess/session_cache.go | 9 ++++++--- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go index 13ed6f568e..45f4a1d32f 100644 --- a/go/libraries/doltcore/sqle/database.go +++ b/go/libraries/doltcore/sqle/database.go @@ -1042,8 +1042,12 @@ func (db Database) tableInsensitive(ctx *sql.Context, root doltdb.RootValue, tab return doltdb.TableName{}, nil, false, fmt.Errorf("no state for database %s", db.RevisionQualifiedName()) } - if tableListKey := root.TableListHash(); tableListKey != 0 { - tableList, ok := dbState.SessionCache().GetCachedTableMap(tableListKey) + tableListKey := root.TableListHash() + + // TODO: refactor to make caching logic more obvious, cache and then retrieve + // TODO: why would tableListKey ever be zero? + if tableListKey != 0 { + tableList, ok := dbState.SessionCache().GetTableNameMap(tableListKey) if ok { tname, ok := tableList[strings.ToLower(tableName)] if ok { @@ -1059,17 +1063,17 @@ func (db Database) tableInsensitive(ctx *sql.Context, root doltdb.RootValue, tab } } - tableNames, err := db.getAllTableNames(ctx, root, true) + tableNames, err := db.getAllTableNames(ctx, root, false) if err != nil { return doltdb.TableName{}, nil, false, err } - if tableListKey := root.TableListHash(); tableListKey != 0 { + if tableListKey != 0 { tableMap := make(map[string]string) for _, table := range tableNames { tableMap[strings.ToLower(table)] = table } - dbState.SessionCache().CacheTableMap(tableListKey, tableMap) + dbState.SessionCache().CacheTableNameMap(tableListKey, tableMap) } tableName, ok = sql.GetTableNameInsensitive(tableName, tableNames) @@ -1150,7 +1154,7 @@ func (db Database) GetAllTableNames(ctx *sql.Context, showSystemTables bool) ([] return db.getAllTableNames(ctx, root, showSystemTables) } -func (db Database) getAllTableNames(ctx *sql.Context, root doltdb.RootValue, showSystemTables bool) ([]string, error) { +func (db Database) getAllTableNames(ctx *sql.Context, root doltdb.RootValue, includeGeneratedSystemTables bool) ([]string, error) { var err error var result []string // If we are in a schema-enabled session and the schema name is not set, we need to union all table names in all @@ -1170,7 +1174,9 @@ func (db Database) getAllTableNames(ctx *sql.Context, root doltdb.RootValue, sho } } - if showSystemTables { + if includeGeneratedSystemTables { + // TODO: this should work on the current schema only, if there is one + // TODO: this is getting called with showSystemTables = true, which seems wrong in most cases systemTables, err := resolve.GetGeneratedSystemTables(ctx, root) if err != nil { return nil, err diff --git a/go/libraries/doltcore/sqle/dsess/session_cache.go b/go/libraries/doltcore/sqle/dsess/session_cache.go index e681f65113..fd34977c33 100644 --- a/go/libraries/doltcore/sqle/dsess/session_cache.go +++ b/go/libraries/doltcore/sqle/dsess/session_cache.go @@ -261,13 +261,16 @@ func (c *SessionCache) CacheTableChecks(key doltdb.DataCacheKey, checks []sql.Ch c.checks[key] = checks } -func (c *SessionCache) GetCachedTableMap(key uint64) (map[string]string, bool) { +// GetTableNameMap returns the cached names of tables in a given root, keyed by their lower-case name, to their +// case-sensitive name +func (c *SessionCache) GetTableNameMap(key uint64) (map[string]string, bool) { tables, ok := c.tableMaps[key] return tables, ok } -// CacheTableChecks caches sql.CheckConstraints for the table named -func (c *SessionCache) CacheTableMap(key uint64, tables map[string]string) { +// CacheTableNameMap caches the names of tables in a given root, keyed by their lower-case name, to their +// case-sensitive name +func (c *SessionCache) CacheTableNameMap(key uint64, tables map[string]string) { c.mu.Lock() defer c.mu.Unlock()