fix(mariadb): address code review — CycleOption enum, CACHE validation, structured errors

- Replace Cycle bool + NoCycle bool with CycleOption enum to prevent
  invalid state where both could be true simultaneously
- Add validation in parseSequenceOptions for contradictory CACHE/NOCACHE
- Replace fmt.Errorf with p.expectedError() for consistent error style

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

Author: 2 people

pkg/sql/ast/ast.go

@@ -1841,6 +1841,18 @@ func (r ReplaceStatement) Children() []Node {
 
 // ── MariaDB SEQUENCE DDL (10.3+) ───────────────────────────────────────────
 
+// CycleOption represents the CYCLE behavior for a sequence.
+type CycleOption int
+
+const (
+	// CycleUnspecified means no CYCLE or NOCYCLE clause was given (database default applies).
+	CycleUnspecified CycleOption = iota
+	// CycleBehavior means CYCLE — sequence wraps around when it reaches min/max.
+	CycleBehavior
+	// NoCycleBehavior means NOCYCLE / NO CYCLE — sequence errors on overflow.
+	NoCycleBehavior
+)
+
 // SequenceOptions holds configuration for CREATE SEQUENCE and ALTER SEQUENCE.
 // Fields are pointers so that unspecified options are distinguishable from zero values.
 type SequenceOptions struct {
@@ -1849,8 +1861,7 @@ type SequenceOptions struct {
 	MinValue    *LiteralValue // MINVALUE n or nil when NO MINVALUE
 	MaxValue    *LiteralValue // MAXVALUE n or nil when NO MAXVALUE
 	Cache       *LiteralValue // CACHE n or nil when NO CACHE / NOCACHE
-	Cycle       bool          // CYCLE
-	NoCycle     bool          // NO CYCLE / NOCYCLE (explicit; default is NO CYCLE)
+	CycleMode   CycleOption   // CYCLE / NOCYCLE / NO CYCLE (CycleUnspecified if not specified)
 	NoCache     bool          // NOCACHE (explicit; Cache=nil alone is ambiguous)
 	Restart     bool          // bare RESTART (reset to start value)
 	RestartWith *LiteralValue // RESTART WITH n (explicit restart value)

pkg/sql/ast/ast_sequence_test.go

@@ -46,7 +46,7 @@ func TestCreateSequenceStatement_ToSQL(t *testing.T) {
 					MinValue:    &ast.LiteralValue{Value: "1"},
 					MaxValue:    &ast.LiteralValue{Value: "9999"},
 					Cache:       &ast.LiteralValue{Value: "100"},
-					Cycle:       true,
+					CycleMode:   ast.CycleBehavior,
 				},
 			},
 			want: "CREATE SEQUENCE s START WITH 1 INCREMENT BY 1 MINVALUE 1 MAXVALUE 9999 CACHE 100 CYCLE",
@@ -55,7 +55,7 @@ func TestCreateSequenceStatement_ToSQL(t *testing.T) {
 			name: "nocycle",
 			stmt: &ast.CreateSequenceStatement{
 				Name:    &ast.Identifier{Name: "s"},
-				Options: ast.SequenceOptions{NoCycle: true},
+				Options: ast.SequenceOptions{CycleMode: ast.NoCycleBehavior},
 			},
 			want: "CREATE SEQUENCE s NOCYCLE",
 		},

pkg/sql/ast/sql.go

@@ -1672,9 +1672,10 @@ func writeSequenceOptions(b *strings.Builder, opts SequenceOptions) {
 	} else if opts.NoCache {
 		b.WriteString(" NOCACHE")
 	}
-	if opts.Cycle {
+	switch opts.CycleMode {
+	case CycleBehavior:
 		b.WriteString(" CYCLE")
-	} else if opts.NoCycle {
+	case NoCycleBehavior:
 		b.WriteString(" NOCYCLE")
 	}
 	if opts.RestartWith != nil {

pkg/sql/parser/mariadb.go

@@ -199,19 +199,19 @@ func (p *Parser) parseSequenceOptions() (ast.SequenceOptions, error) {
 			case "MAXVALUE":
 				opts.MaxValue = nil
 			case "CYCLE":
-				opts.NoCycle = true
+				opts.CycleMode = ast.NoCycleBehavior
 			case "CACHE":
 				opts.Cache = nil
 				opts.NoCache = true
 			default:
-				return opts, fmt.Errorf("unexpected token after NO in SEQUENCE options: %s", sub)
+				return opts, p.expectedError("MINVALUE, MAXVALUE, CYCLE, or CACHE after NO")
 			}
 		case "CYCLE":
 			p.advance()
-			opts.Cycle = true
+			opts.CycleMode = ast.CycleBehavior
 		case "NOCYCLE":
 			p.advance()
-			opts.NoCycle = true
+			opts.CycleMode = ast.NoCycleBehavior
 		case "CACHE":
 			p.advance()
 			lit, err := p.parseNumericLit()
@@ -238,6 +238,10 @@ func (p *Parser) parseSequenceOptions() (ast.SequenceOptions, error) {
 			return opts, nil
 		}
 	}
+	// Validate: CACHE n and NOCACHE are mutually exclusive.
+	if opts.Cache != nil && opts.NoCache {
+		return opts, fmt.Errorf("contradictory sequence options: CACHE and NOCACHE cannot both be specified")
+	}
 	return opts, nil
 }
 

pkg/sql/parser/mariadb_test.go

@@ -40,8 +40,8 @@ func TestMariaDB_CreateSequence_AllOptions(t *testing.T) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 	stmt := tree.Statements[0].(*ast.CreateSequenceStatement)
-	if !stmt.Options.Cycle {
-		t.Error("expected Cycle = true")
+	if stmt.Options.CycleMode != ast.CycleBehavior {
+		t.Error("expected CycleMode = CycleBehavior")
 	}
 	if stmt.Options.Cache == nil {
 		t.Error("expected Cache to be set")