optimize: ddl operators don't need infoschema

.golangci.yml

@@ -50,7 +50,7 @@ formatters:
     goimports:
       # put imports beginning with prefix after 3rd-party packages;
       # it's a comma-separated list of prefixes
-      local-prefixes: 
+      local-prefixes:
         - github.com/secretflow/scql
 
 linters:
@@ -68,7 +68,7 @@ linters:
         linters: [govet]
   settings:
     staticcheck:
-      # All supported checks can be enabled with "all". 
+      # All supported checks can be enabled with "all".
       # To disable checks, prefix them with a minus sign.
       # Example: [ "all", "-SA1000", "-SA1001"]
       checks: ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-QF1003", "-QF1006", "-QF1008"]

pkg/planner/core/preprocess.go

@@ -60,6 +60,8 @@ const (
 	parentIsJoin
 	// inRepairTable is set when visiting a repair table statement.
 	inRepairTable
+	// inShowColumns is set when visiting SHOW COLUMNS/DESCRIBE statement.
+	inShowColumns
 )
 
 // preprocessor is an ast.Visitor that preprocess
@@ -88,6 +90,9 @@ func (p *preprocessor) Enter(in ast.Node) (out ast.Node, skipChildren bool) {
 		p.flag |= inCreateOrDropTable
 		//p.checkDropTableGrammar(node)
 	case *ast.ShowStmt:
+		if node.Tp == ast.ShowColumns {
+			p.flag |= inShowColumns
+		}
 		p.resolveShowStmt(node)
 	case *ast.UnionSelectList:
 		p.checkUnionSelectList(node)
@@ -107,6 +112,10 @@ func (p *preprocessor) Leave(in ast.Node) (out ast.Node, ok bool) {
 		//p.checkContainDotColumn(x)
 	case *ast.DropTableStmt, *ast.AlterTableStmt, *ast.RenameTableStmt:
 		p.flag &= ^inCreateOrDropTable
+	case *ast.ShowStmt:
+		if x.Tp == ast.ShowColumns {
+			p.flag &= ^inShowColumns
+		}
 	case *ast.TableName:
 		p.handleTableName(x)
 	case *ast.Join:
@@ -146,8 +155,9 @@ func (p *preprocessor) handleTableName(tn *ast.TableName) {
 		}
 		tn.Schema = model.NewCIStr(currentDB)
 	}
-	if p.flag&inCreateOrDropTable > 0 {
-		// The table may not exist in create table or drop table statement.
+	if p.flag&(inCreateOrDropTable|inShowColumns) > 0 {
+		// The table may not exist in create/drop table statement.
+		// For SHOW COLUMNS, we don't need InfoSchema in preprocess, will query storage directly in executor.
 		return
 	}
 

pkg/scdb/executor/ddl.go

@@ -199,13 +199,14 @@ func (e *DDLExec) executeCreateTable(s *ast.CreateTableStmt) (err error) {
 	if len(lowerColumnNames) != len(sliceutil.SliceDeDup(lowerColumnNames)) {
 		return fmt.Errorf("ddl.executeCreateTable: duplicate column names in table %s", tblName)
 	}
-	for _, c := range s.Cols {
+	for i, c := range s.Cols {
 		// TODO: fill description field
 		result = tx.Create(&storage.Column{
-			Db:         dbName,
-			TableName:  tblName,
-			ColumnName: c.Name.String(),
-			Type:       c.Type,
+			Db:              dbName,
+			TableName:       tblName,
+			ColumnName:      c.Name.String(),
+			OrdinalPosition: uint(i),
+			Type:            strings.ToLower(c.Type),
 		})
 		if result.Error != nil {
 			return fmt.Errorf("ddl.executeCreateTable: %v", result.Error)

pkg/scdb/executor/executor_test.go

@@ -39,7 +39,6 @@ import (
 	"github.com/secretflow/scql/pkg/scdb/storage"
 	"github.com/secretflow/scql/pkg/sessionctx"
 	"github.com/secretflow/scql/pkg/sessionctx/stmtctx"
-	"github.com/secretflow/scql/pkg/types"
 )
 
 type userAuth struct {
@@ -1439,43 +1438,6 @@ func TestShow(t *testing.T) {
 }
 
 func TestDescribe(t *testing.T) {
-	is := infoschema.MockInfoSchema(
-		map[string][]*model.TableInfo{
-			"da": {
-				{
-					Name: model.NewCIStr("t1"),
-					Columns: []*model.ColumnInfo{
-						{
-							Name:      model.NewCIStr("c1"),
-							FieldType: *types.NewFieldType(mysql.TypeString),
-							Comment:   "string value",
-						},
-						{
-							Name:      model.NewCIStr("c2"),
-							FieldType: *types.NewFieldType(mysql.TypeLong),
-							Comment:   "long value",
-						},
-					},
-				},
-			},
-			"db": {
-				{
-					Name: model.NewCIStr("t1"),
-					Columns: []*model.ColumnInfo{
-						{
-							Name:      model.NewCIStr("c1"),
-							FieldType: *types.NewFieldType(mysql.TypeString),
-							Comment:   "string value",
-						},
-						{
-							Name:      model.NewCIStr("c2"),
-							FieldType: *types.NewFieldType(mysql.TypeLong),
-							Comment:   "long value",
-						},
-					},
-				},
-			},
-		})
 	r := require.New(t)
 	ctx := setupTestEnv(t)
 
@@ -1496,28 +1458,30 @@ func TestDescribe(t *testing.T) {
 	_, err = runSQL(ctx, `GRANT DESCRIBE ON db.* to bob`)
 	r.NoError(err)
 
+	// Root creates db.t1 for bob to describe later
+	_, err = runSQL(ctx, `CREATE TABLE db.t1 (c1 int, c2 int) REF_TABLE=d1.t1 DB_TYPE='mysql'`)
+	r.NoError(err)
+
 	// switch to user alice
 	r.NoError(switchUser(ctx, userAlice))
 
 	_, err = runSQL(ctx, `CREATE TABLE da.t1 (c1 int, c2 int) REF_TABLE=d1.t1 DB_TYPE='mysql'`)
 	r.NoError(err)
 
-	// alice>
-	rt, err := Run(ctx, `DESCRIBE da.t1`, is)
+	// alice> DESCRIBE uses storage directly, no need for InfoSchema
+	rt, err := runSQL(ctx, `DESCRIBE da.t1`)
 	r.NoError(err)
 	r.Equal(int64(2), rt[0].GetShape().GetDim()[0].GetDimValue())
 
-	_, err = Run(ctx, `DESCRIBE db.t1`, is)
-	r.Error(err)
-
 	// alice> (switch to user bob)
 	r.NoError(switchUser(ctx, userBob))
-	// bob>
-	rt, err = Run(ctx, `DESCRIBE db.t1`, is)
+	// bob> can describe db.t1
+	rt, err = runSQL(ctx, `DESCRIBE db.t1`)
 	r.NoError(err)
 	r.Equal(int64(2), rt[0].GetShape().GetDim()[0].GetDimValue())
 
-	_, err = Run(ctx, `DESCRIBE da.t1`, is)
+	// bob cannot describe da.t1 (no DESCRIBE privilege)
+	_, err = runSQL(ctx, `DESCRIBE da.t1`)
 	r.Error(err)
 }
 
@@ -1701,32 +1665,15 @@ func TestDescribeTable(t *testing.T) {
 	r := require.New(t)
 	ctx := setupTestEnv(t)
 
-	is := infoschema.MockInfoSchema(
-		map[string][]*model.TableInfo{
-			"da": {
-				{
-					Name: model.NewCIStr("t1"),
-					Columns: []*model.ColumnInfo{
-						{
-							Name:      model.NewCIStr("c1"),
-							FieldType: *types.NewFieldType(mysql.TypeString),
-							Comment:   "string value",
-						},
-						{
-							Name:      model.NewCIStr("c2"),
-							FieldType: *types.NewFieldType(mysql.TypeLong),
-							Comment:   "long value",
-						},
-					},
-				},
-			},
-		})
-
-	// fail due to scdb not support explain stmt
-	_, err := Run(ctx, "EXPLAIN SELECT * from da.t1", is)
-	r.Error(err)
+	// Create database and table in storage
+	_, err := runSQL(ctx, `CREATE DATABASE da`)
+	r.NoError(err)
+
+	_, err = runSQL(ctx, `CREATE TABLE da.t1 (c1 string, c2 int) REF_TABLE=d1.t1 DB_TYPE='mysql'`)
+	r.NoError(err)
 
-	result, err := Run(ctx, "DESCRIBE da.t1", is)
+	// DESCRIBE now queries storage directly, no need for InfoSchema
+	result, err := runSQL(ctx, "DESCRIBE da.t1")
 	r.NoError(err)
 	r.Equal(2, len(result))
 
@@ -1735,6 +1682,7 @@ func TestDescribeTable(t *testing.T) {
 
 	r.Equal("Type", result[1].GetName())
 	r.Equal(2, len(result[1].GetStringData()))
+	// Type should be lowercase as stored in storage
 	r.ElementsMatch([]string{"string", "int"}, result[1].GetStringData())
 }
 

pkg/scdb/executor/show.go

@@ -34,7 +34,6 @@ import (
 	"github.com/secretflow/scql/pkg/privilege"
 	"github.com/secretflow/scql/pkg/scdb/storage"
 	"github.com/secretflow/scql/pkg/sessionctx"
-	"github.com/secretflow/scql/pkg/table"
 	"github.com/secretflow/scql/pkg/types"
 	"github.com/secretflow/scql/pkg/util/chunk"
 	"github.com/secretflow/scql/pkg/util/mathutil"
@@ -427,16 +426,17 @@ func getAllDatabaseNames(ctx sessionctx.Context) ([]string, error) {
 }
 
 func (e *ShowExec) fetchShowColumns(ctx context.Context) error {
-	tb, err := e.getTable()
-
-	if err != nil {
-		return errors.Trace(err)
+	if e.Table == nil {
+		return errors.New("table not found")
 	}
 
+	dbName := e.Table.Schema.O
+	tableName := e.Table.Name.O
+
 	// check database is visible
 	checker := privilege.GetPrivilegeManager(e.ctx)
 	if checker != nil && e.ctx.GetSessionVars().User != nil {
-		ok, err := checker.DBIsVisible(e.ctx.GetSessionVars().ActiveRoles, e.DBName.O, mysql.DescribePriv)
+		ok, err := checker.DBIsVisible(e.ctx.GetSessionVars().ActiveRoles, dbName, mysql.DescribePriv)
 		if err != nil {
 			return fmt.Errorf("showExec.fetchShowColumns: %v", err)
 		}
@@ -445,45 +445,44 @@ func (e *ShowExec) fetchShowColumns(ctx context.Context) error {
 		}
 	}
 
-	for _, col := range tb.Cols() {
-		if e.Column != nil && e.Column.Name.String() != col.Name.String() {
-			continue
-		}
+	// Query columns directly from storage instead of using InfoSchema
+	db := e.ctx.GetSessionVars().Storage
+	var columns []storage.Column
+	query := db.Model(&storage.Column{}).Where(&storage.Column{
+		Db:        dbName,
+		TableName: tableName,
+	})
 
-		tpStr, err := infoschema.FieldTypeString(col.FieldType)
-		if err != nil {
-			return fmt.Errorf("fail to convert mysql field type %v to scdb field type: %v", col.FieldType, err)
-		}
+	// If specific column is requested
+	if e.Column != nil {
+		query = query.Where("column_name = ?", e.Column.Name.String())
+	}
 
+	// Order by ordinal_position
+	err := query.Order("ordinal_position").Find(&columns).Error
+	if err != nil {
+		return errors.Wrapf(err, "showExec.fetchShowColumns: failed to query columns")
+	}
+
+	for _, col := range columns {
 		// The FULL keyword causes the output to include the column collation and comments,
 		// as well as the privileges you have for each column.
 		if e.Full {
 			e.appendRow([]interface{}{
-				col.Name.String(),
-				tpStr,
-				col.Comment,
+				col.ColumnName,
+				col.Type,
+				col.Description,
 			})
 		} else {
 			e.appendRow([]interface{}{
-				col.Name.String(),
-				tpStr,
+				col.ColumnName,
+				col.Type,
 			})
 		}
 	}
 	return nil
 }
 
-func (e *ShowExec) getTable() (table.Table, error) {
-	if e.Table == nil {
-		return nil, errors.New("table not found")
-	}
-	tb, err := e.is.TableByName(e.Table.Schema, e.Table.Name)
-	if err != nil {
-		return nil, err
-	}
-	return tb, nil
-}
-
 func (e *ShowExec) dbAccessDenied() error {
 	user := e.ctx.GetSessionVars().User
 	u := user.Username

pkg/scdb/server/submit_handler.go

@@ -158,9 +158,39 @@ func isQueryNeedInfoSchema(query string) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	switch stmt.(type) {
-	case *ast.CreateUserStmt, *ast.DropTableStmt:
+	switch s := stmt.(type) {
+	case *ast.CreateUserStmt, *ast.DropUserStmt, *ast.AlterUserStmt:
+		// User management statements only operate on users table
+		// They use storage.CheckUserExist() directly, no need for InfoSchema
 		return false, nil
+	case *ast.GrantStmt, *ast.RevokeStmt:
+		// GRANT and REVOKE statements have their own validation logic
+		// They directly query storage to check existence of db/table/columns
+		// No need to load complete InfoSchema
+		return false, nil
+	case *ast.CreateDatabaseStmt, *ast.DropDatabaseStmt:
+		// CREATE/DROP DATABASE only need to check database existence
+		// They use storage.CheckDatabaseExist() directly, no need for InfoSchema
+		return false, nil
+	case *ast.CreateTableStmt, *ast.DropTableStmt:
+		// CREATE/DROP TABLE only need to check existence
+		// They use CheckDatabaseExist() and CheckTableExist() directly, no need for InfoSchema
+		return false, nil
+	case *ast.ShowStmt:
+		// SHOW DATABASES, SHOW TABLES, SHOW GRANTS don't need InfoSchema
+		// SHOW COLUMNS can query storage.Column table directly
+		switch s.Tp {
+		case ast.ShowDatabases, ast.ShowTables, ast.ShowGrants, ast.ShowColumns:
+			return false, nil
+		default:
+			return true, nil
+		}
+	case *ast.ExplainStmt:
+		// DESCRIBE/DESC table is parsed as ExplainStmt with ShowStmt inside
+		if showStmt, ok := s.Stmt.(*ast.ShowStmt); ok && showStmt.Tp == ast.ShowColumns {
+			return false, nil
+		}
+		return true, nil
 	default:
 		return true, nil
 	}