fix(mariadb): code review fixes — pool comments, expressionNode docs, PERIOD FOR SYSTEM_TIME test

- Add '// zero all fields' comment to ReleaseCreateSequenceStatement and ReleaseAlterSequenceStatement
- Add design-choice comments on expressionNode() for ForSystemTimeClause, ConnectByClause, PeriodDefinition
- Add TestMariaDB_CreateTable_PeriodForSystemTime to cover PERIOD FOR SYSTEM_TIME parsing
- Fix parsePeriodDefinition to use parseColumnName so SYSTEM_TIME (reserved keyword) is accepted as period name
- Add GENERATED ALWAYS AS ROW START/END column constraint parsing in parseColumnConstraint (MariaDB system-versioned columns)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: 2 people

pkg/sql/ast/ast.go

@@ -1938,6 +1938,9 @@ type ForSystemTimeClause struct {
 	Pos   models.Location // Source position of the FOR keyword (1-based line and column)
 }
 
+// expressionNode satisfies the Expression interface so ForSystemTimeClause can be
+// stored in TableReference.ForSystemTime without a separate interface type.
+// Semantically it is a table-level clause, not a scalar expression.
 func (c *ForSystemTimeClause) expressionNode()     {}
 func (c ForSystemTimeClause) TokenLiteral() string { return "FOR SYSTEM_TIME" }
 func (c ForSystemTimeClause) Children() []Node {
@@ -1965,6 +1968,9 @@ type PeriodDefinition struct {
 	Pos      models.Location // Source position of the PERIOD FOR keyword (1-based line and column)
 }
 
+// expressionNode satisfies the Expression interface so PeriodDefinition can be
+// stored in CreateTableStatement.PeriodDefinitions without a separate interface type.
+// Semantically it is a table column constraint, not a scalar expression.
 func (p *PeriodDefinition) expressionNode()     {}
 func (p PeriodDefinition) TokenLiteral() string { return "PERIOD FOR" }
 func (p PeriodDefinition) Children() []Node {
@@ -1994,6 +2000,9 @@ type ConnectByClause struct {
 	Pos       models.Location // Source position of the CONNECT BY keyword (1-based line and column)
 }
 
+// expressionNode satisfies the Expression interface so ConnectByClause can be
+// stored in SelectStatement.ConnectBy without a separate interface type.
+// Semantically it is a query-level clause, not a scalar expression.
 func (c *ConnectByClause) expressionNode()     {}
 func (c ConnectByClause) TokenLiteral() string { return "CONNECT BY" }
 func (c ConnectByClause) Children() []Node {

pkg/sql/ast/pool.go

@@ -1814,7 +1814,7 @@ func NewCreateSequenceStatement() *CreateSequenceStatement {
 
 // ReleaseCreateSequenceStatement returns a CreateSequenceStatement to the pool.
 func ReleaseCreateSequenceStatement(s *CreateSequenceStatement) {
-	*s = CreateSequenceStatement{}
+	*s = CreateSequenceStatement{} // zero all fields
 	createSequencePool.Put(s)
 }
 
@@ -1837,6 +1837,6 @@ func NewAlterSequenceStatement() *AlterSequenceStatement {
 
 // ReleaseAlterSequenceStatement returns an AlterSequenceStatement to the pool.
 func ReleaseAlterSequenceStatement(s *AlterSequenceStatement) {
-	*s = AlterSequenceStatement{}
+	*s = AlterSequenceStatement{} // zero all fields
 	alterSequencePool.Put(s)
 }

pkg/sql/parser/ddl_columns.go

@@ -18,6 +18,8 @@
 package parser
 
 import (
+	"strings"
+
 	goerrors "github.com/ajitpratap0/GoSQLX/pkg/errors"
 	"github.com/ajitpratap0/GoSQLX/pkg/models"
 	"github.com/ajitpratap0/GoSQLX/pkg/sql/ast"
@@ -247,6 +249,34 @@ func (p *Parser) parseColumnConstraint() (*ast.ColumnConstraint, bool, error) {
 		return constraint, true, nil
 	}
 
+	// GENERATED ALWAYS AS ROW START / ROW END (MariaDB system-versioned columns)
+	// Syntax: GENERATED ALWAYS AS ROW START | ROW END
+	if strings.EqualFold(p.currentToken.Token.Value, "GENERATED") {
+		p.advance() // Consume GENERATED
+		// Optional ALWAYS
+		if strings.EqualFold(p.currentToken.Token.Value, "ALWAYS") {
+			p.advance() // Consume ALWAYS
+		}
+		// Expect AS
+		if !p.isType(models.TokenTypeAs) {
+			return nil, false, p.expectedError("AS after GENERATED [ALWAYS]")
+		}
+		p.advance() // Consume AS
+		// Expect ROW
+		if !p.isType(models.TokenTypeRow) {
+			return nil, false, p.expectedError("ROW after GENERATED [ALWAYS] AS")
+		}
+		p.advance() // Consume ROW
+		// Expect START or END
+		rowRole := strings.ToUpper(p.currentToken.Token.Value)
+		if rowRole != "START" && rowRole != "END" {
+			return nil, false, p.expectedError("START or END after GENERATED [ALWAYS] AS ROW")
+		}
+		p.advance() // Consume START or END
+		constraint.Type = "GENERATED ALWAYS AS ROW " + rowRole
+		return constraint, true, nil
+	}
+
 	// No constraint found
 	return nil, false, nil
 }

pkg/sql/parser/mariadb.go

@@ -441,7 +441,8 @@ func (p *Parser) parsePeriodDefinition() (*ast.PeriodDefinition, error) {
 	}
 	p.advance()
 
-	name := p.parseIdent()
+	// Use parseColumnName so that reserved-keyword period names like SYSTEM_TIME are accepted.
+	name := p.parseColumnName()
 	if name == nil || name.Name == "" {
 		return nil, p.expectedError("period name")
 	}

pkg/sql/parser/mariadb_test.go

@@ -318,3 +318,39 @@ func TestMariaDB_SQLFiles(t *testing.T) {
 		})
 	}
 }
+
+func TestMariaDB_CreateTable_PeriodForSystemTime(t *testing.T) {
+	sql := `CREATE TABLE t (
+		id INT,
+		row_start DATETIME(6) GENERATED ALWAYS AS ROW START,
+		row_end   DATETIME(6) GENERATED ALWAYS AS ROW END,
+		PERIOD FOR SYSTEM_TIME(row_start, row_end)
+	) WITH SYSTEM VERSIONING`
+	tree, err := parser.ParseWithDialect(sql, keywords.DialectMariaDB)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(tree.Statements) != 1 {
+		t.Fatalf("expected 1 statement, got %d", len(tree.Statements))
+	}
+	stmt, ok := tree.Statements[0].(*ast.CreateTableStatement)
+	if !ok {
+		t.Fatalf("expected CreateTableStatement, got %T", tree.Statements[0])
+	}
+	if len(stmt.PeriodDefinitions) == 0 {
+		t.Fatal("expected at least one PeriodDefinition")
+	}
+	pd := stmt.PeriodDefinitions[0]
+	if pd.Name == nil || !strings.EqualFold(pd.Name.Name, "SYSTEM_TIME") {
+		t.Errorf("expected period name SYSTEM_TIME, got %v", pd.Name)
+	}
+	if pd.StartCol == nil || pd.StartCol.Name != "row_start" {
+		t.Errorf("expected StartCol=row_start, got %v", pd.StartCol)
+	}
+	if pd.EndCol == nil || pd.EndCol.Name != "row_end" {
+		t.Errorf("expected EndCol=row_end, got %v", pd.EndCol)
+	}
+	if !stmt.WithSystemVersioning {
+		t.Error("expected WithSystemVersioning = true")
+	}
+}