fix(oracle): require ';' after PL/SQL statements, declarations, and END (BYT-10301)

oracle/ast/loc.go

@@ -485,6 +485,8 @@ func NodeLoc(n Node) Loc {
 		return v.Loc
 	case *PLSQLTypeDecl:
 		return v.Loc
+	case *PLSQLSubtypeDecl:
+		return v.Loc
 	case *PLSQLAssign:
 		return v.Loc
 	case *PLSQLIf:

oracle/ast/outfuncs.go

@@ -589,6 +589,8 @@ func writeNode(sb *strings.Builder, node Node) {
 		writePLSQLWhen(sb, n)
 	case *PLSQLTypeDecl:
 		writePLSQLTypeDecl(sb, n)
+	case *PLSQLSubtypeDecl:
+		writePLSQLSubtypeDecl(sb, n)
 	case *PLSQLCall:
 		writePLSQLCall(sb, n)
 
@@ -5344,6 +5346,9 @@ func writePLSQLTypeDecl(sb *strings.Builder, n *PLSQLTypeDecl) {
 		sb.WriteString(" :elementType ")
 		writeNode(sb, n.ElementType)
 	}
+	if n.ElementNotNull {
+		sb.WriteString(" :elementNotNull true")
+	}
 	if n.IndexBy != nil {
 		sb.WriteString(" :indexBy ")
 		writeNode(sb, n.IndexBy)
@@ -5364,6 +5369,28 @@ func writePLSQLTypeDecl(sb *strings.Builder, n *PLSQLTypeDecl) {
 	sb.WriteString("}")
 }
 
+func writePLSQLSubtypeDecl(sb *strings.Builder, n *PLSQLSubtypeDecl) {
+	sb.WriteString("{PLSQLSUBTYPEDECL")
+	sb.WriteString(fmt.Sprintf(" :name %q", n.Name))
+	if n.BaseType != nil {
+		sb.WriteString(" :baseType ")
+		writeNode(sb, n.BaseType)
+	}
+	if n.RangeLow != nil {
+		sb.WriteString(" :rangeLow ")
+		writeNode(sb, n.RangeLow)
+	}
+	if n.RangeHigh != nil {
+		sb.WriteString(" :rangeHigh ")
+		writeNode(sb, n.RangeHigh)
+	}
+	if n.NotNull {
+		sb.WriteString(" :notNull true")
+	}
+	sb.WriteString(fmt.Sprintf(" :loc_start %d :loc_end %d", n.Loc.Start, n.Loc.End))
+	sb.WriteString("}")
+}
+
 func writePLSQLCall(sb *strings.Builder, n *PLSQLCall) {
 	sb.WriteString("{PLSQLCALL")
 	if n.Name != nil {

oracle/ast/parsenodes.go

@@ -2561,7 +2561,8 @@ type PLSQLPragma struct {
 	Loc  Loc
 }
 
-func (n *PLSQLPragma) nodeTag() {}
+func (n *PLSQLPragma) nodeTag()  {}
+func (n *PLSQLPragma) stmtNode() {} // INLINE and COVERAGE also stand as statements
 
 // PLSQLCase represents a PL/SQL CASE statement (distinct from CASE expression).
 type PLSQLCase struct {
@@ -2590,18 +2591,33 @@ func (n *PLSQLWhen) nodeTag() {}
 //	TYPE name IS RECORD (field type [,...])
 //	TYPE name IS REF CURSOR [RETURN type]
 type PLSQLTypeDecl struct {
-	Name        string    // type name
-	Kind        string    // TABLE, VARRAY, RECORD, REF_CURSOR
-	ElementType *TypeName // element type (TABLE OF/VARRAY OF)
-	IndexBy     *TypeName // INDEX BY type (associative arrays)
-	Limit       ExprNode  // VARRAY limit
-	Fields      *List     // RECORD fields
-	ReturnType  *TypeName // REF CURSOR RETURN type
-	Loc         Loc
+	Name           string    // type name
+	Kind           string    // TABLE, VARRAY, RECORD, REF_CURSOR
+	ElementType    *TypeName // element type (TABLE OF/VARRAY OF)
+	ElementNotNull bool      // TABLE OF/VARRAY OF type NOT NULL
+	IndexBy        *TypeName // INDEX BY type (associative arrays)
+	Limit          ExprNode  // VARRAY limit
+	Fields         *List     // RECORD fields
+	ReturnType     *TypeName // REF CURSOR RETURN type
+	Loc            Loc
 }
 
 func (n *PLSQLTypeDecl) nodeTag() {}
 
+// PLSQLSubtypeDecl represents a PL/SQL SUBTYPE declaration.
+//
+//	SUBTYPE name IS base_type [RANGE low .. high] [NOT NULL]
+type PLSQLSubtypeDecl struct {
+	Name      string    // subtype name
+	BaseType  *TypeName // base type, with its precision, scale, or size
+	RangeLow  ExprNode  // RANGE lower bound, nil without RANGE
+	RangeHigh ExprNode  // RANGE upper bound, nil without RANGE
+	NotNull   bool      // NOT NULL
+	Loc       Loc
+}
+
+func (n *PLSQLSubtypeDecl) nodeTag() {}
+
 // PLSQLCall represents a PL/SQL standalone procedure call statement.
 type PLSQLCall struct {
 	Name ExprNode // procedure/function reference (could be dotted name)

oracle/parser/create_proc.go

@@ -462,12 +462,13 @@ func (p *Parser) parseCreatePackageStmt(start int, orReplace, ifNotExists, editi
 	}
 	p.advance() // consume END
 	// Optional package name after END
-	if p.isIdentLike() && p.cur.Type != ';' && p.cur.Type != tokEOF {
+	if p.atEndName() {
 		p.advance() // consume name
 	}
-	if p.cur.Type == ';' {
-		p.advance() // consume ;
+	if p.cur.Type != ';' {
+		return nil, p.syntaxErrorAtCur()
 	}
+	p.advance() // consume ;
 
 	stmt.Loc.End = p.prev.End
 	return stmt, nil
@@ -479,12 +480,6 @@ func (p *Parser) parsePackageBody() (*nodes.List, error) {
 	decls := &nodes.List{}
 
 	for p.cur.Type != kwEND && p.cur.Type != tokEOF {
-		// Skip standalone semicolons
-		if p.cur.Type == ';' {
-			p.advance()
-			continue
-		}
-
 		// PROCEDURE declaration/definition in package
 		if p.cur.Type == kwPROCEDURE {
 			decl, parseErr470 := p.parsePackageProcDecl()
@@ -514,18 +509,14 @@ func (p *Parser) parsePackageBody() (*nodes.List, error) {
 			break
 		}
 
-		// Variable/type/cursor declarations
+		// Variable/type/cursor declarations. Each ends with ';', and a
+		// stray ';' between items is PLS-00103 on Oracle 23ai.
 		decl, parseErr472 := p.parsePLSQLDeclaration()
 		if parseErr472 != nil {
-			return nil,
-
-				// If we can't parse anything, skip a token to avoid infinite loop
-				parseErr472
+			return nil, parseErr472
 		}
-		if decl == nil {
-
-			p.advance()
-			continue
+		if decl == nil || p.prev.Type != ';' {
+			return nil, p.syntaxErrorAtCur()
 		}
 		decls.Items = append(decls.Items, decl)
 	}

oracle/parser/create_trigger.go

@@ -164,11 +164,16 @@ func (p *Parser) parseCreateTriggerStmt(start int, orReplace, ifNotExists, editi
 				if depth > 0 {
 					depth--
 				} else {
-					// Outer END — consume END, optional name, done.
+					// Outer END — consume END, optional name, and the ';'
+					// that ends the trigger (PLS-00103 without it).
 					p.advance() // consume END
-					if p.isIdentLike() {
+					if p.atEndName() {
 						p.advance() // consume trigger name
 					}
+					if p.cur.Type != ';' {
+						return nil, p.syntaxErrorAtCur()
+					}
+					p.advance() // consume ;
 					break
 				}
 			}

oracle/parser/create_type.go

@@ -153,12 +153,13 @@ func (p *Parser) parseCreateTypeStmt(start int, orReplace, ifNotExists, editiona
 			}
 			p.advance()
 			// Optional type name after END
-			if p.isIdentLike() && p.cur.Type != ';' && p.cur.Type != tokEOF {
+			if p.atEndName() {
 				p.advance()
 			}
-			if p.cur.Type == ';' {
-				p.advance()
+			if p.cur.Type != ';' {
+				return nil, p.syntaxErrorAtCur()
 			}
+			p.advance()
 		}
 	}
 

oracle/parser/create_type_test.go

@@ -80,7 +80,7 @@ func TestParseCreateTypeVarray(t *testing.T) {
 func TestParseCreateTypeBody(t *testing.T) {
 	// A type body holds subprogram definitions and ends with END; Oracle
 	// rejects a body without it (PLS-00103), and so does omni.
-	p := newTestParser("TYPE BODY my_type IS MEMBER FUNCTION f RETURN NUMBER IS BEGIN RETURN 1; END; END my_type")
+	p := newTestParser("TYPE BODY my_type IS MEMBER FUNCTION f RETURN NUMBER IS BEGIN RETURN 1; END; END my_type;")
 	stmt, parseErr5 := p.parseCreateTypeStmt(0, false, false, false, false)
 	if parseErr5 != nil {
 		t.Fatalf("parse: %v", parseErr5)

oracle/parser/keywords.go

@@ -126,6 +126,49 @@ func isOracleContextKeyword(tokenType int) bool {
 	}
 }
 
+// plsqlReservedWords are the words Oracle 23ai rejects, unquoted, where
+// PL/SQL expects a name, such as the label after END (PLS-00103). They were
+// found by compiling BEGIN BEGIN NULL; END word; END; for each word of the
+// documented PL/SQL reserved words, each SQL reserved word, and each PL/SQL
+// statement keyword; the engine, not the list, decides.
+//
+// Ref: https://docs.oracle.com/en/database/oracle/oracle-database/23/lnpls/plsql-reserved-words-keywords.html
+var plsqlReservedWords = map[string]bool{
+	"ALL": true, "ALTER": true, "AND": true, "ANY": true, "AS": true, "ASC": true, "AT": true,
+	"BEGIN": true, "BETWEEN": true, "BY": true, "CASE": true, "CHECK": true, "CLUSTER": true,
+	"CLUSTERS": true, "COLAUTH": true, "COLUMNS": true, "COMPRESS": true, "CONNECT": true,
+	"CRASH": true, "CREATE": true, "CURRENT": true, "DECLARE": true, "DEFAULT": true,
+	"DELETE": true, "DESC": true, "DISTINCT": true, "DROP": true, "ELSE": true, "END": true,
+	"EXCEPTION": true, "EXCLUSIVE": true, "EXISTS": true, "FETCH": true, "FOR": true,
+	"FROM": true, "GOTO": true, "GRANT": true, "GROUP": true, "HAVING": true,
+	"IDENTIFIED": true, "IF": true, "IN": true, "INDEX": true, "INDEXES": true, "INSERT": true,
+	"INTERSECT": true, "INTO": true, "IS": true, "LIKE": true, "LOCK": true, "MINUS": true,
+	"MODE": true, "NOCOMPRESS": true, "NOT": true, "NOWAIT": true, "NULL": true, "OF": true,
+	"ON": true, "OPTION": true, "OR": true, "ORDER": true, "OVERLAPS": true, "PRIOR": true,
+	"PROCEDURE": true, "PUBLIC": true, "RESOURCE": true, "REVOKE": true, "SELECT": true,
+	"SHARE": true, "SIZE": true, "SQL": true, "START": true, "TABAUTH": true, "TABLE": true,
+	"THEN": true, "TO": true, "UNION": true, "UNIQUE": true, "UPDATE": true, "VALUES": true,
+	"VIEW": true, "VIEWS": true, "WHEN": true, "WHERE": true, "WITH": true,
+}
+
+// isPLSQLIdentifier reports whether the current token can name a PL/SQL
+// item: a quoted identifier, or a word that is not PL/SQL reserved. SUBTYPE
+// loop IS NUMBER compiles on Oracle 23ai; SUBTYPE prior IS NUMBER does not.
+func (p *Parser) isPLSQLIdentifier() bool {
+	if p.cur.Type == tokQIDENT {
+		return true
+	}
+	return p.isIdentLike() && !plsqlReservedWords[p.cur.Str]
+}
+
+// atEndName reports whether the current token, just after END or END LOOP,
+// is the optional label or unit name. A PL/SQL reserved word is not: in
+// END LOOP END; the second END closes the enclosing block, and Oracle reports
+// the missing ';' at it.
+func (p *Parser) atEndName() bool {
+	return p.isPLSQLIdentifier()
+}
+
 // isOracleSQLReservedKeyword returns true when tok is an Oracle SQL reserved word
 // that cannot be used as a nonquoted object or column identifier.
 func isOracleSQLReservedKeyword(tok Token) bool {

oracle/parser/lock_table_strict_test.go

@@ -0,0 +1,52 @@
+package parser
+
+import (
+	"testing"
+
+	"github.com/bytebase/omni/oracle/ast"
+)
+
+// TestLockTableClausesStrict checks the lock mode, WAIT interval, and
+// partition forms of LOCK TABLE against what Oracle 23ai accepts (reference
+// rows ref_140 to ref_146).
+func TestLockTableClausesStrict(t *testing.T) {
+	for sql, want := range map[string]string{
+		"LOCK TABLE t IN ROW SHARE MODE":           "ROW SHARE",
+		"LOCK TABLE t IN ROW EXCLUSIVE MODE":       "ROW EXCLUSIVE",
+		"LOCK TABLE t IN SHARE UPDATE MODE":        "SHARE UPDATE",
+		"LOCK TABLE t IN SHARE MODE":               "SHARE",
+		"LOCK TABLE t IN SHARE ROW EXCLUSIVE MODE": "SHARE ROW EXCLUSIVE",
+		"LOCK TABLE t IN EXCLUSIVE MODE WAIT 5":    "EXCLUSIVE",
+	} {
+		result := ParseAndCheck(t, sql)
+		stmt := result.Items[0].(*ast.RawStmt).Stmt.(*ast.LockTableStmt)
+		if stmt.LockMode != want {
+			t.Errorf("Parse(%q) LockMode = %q, want %q", sql, stmt.LockMode, want)
+		}
+	}
+
+	stmt := ParseAndCheck(t, "LOCK TABLE t PARTITION FOR (2023, 'x') IN SHARE MODE").Items[0].(*ast.RawStmt).Stmt.(*ast.LockTableStmt)
+	if got := stmt.Tables[0].PartitionName; got != "2023, 'x'" {
+		t.Errorf("PARTITION FOR key values = %q, want %q", got, "2023, 'x'")
+	}
+
+	for _, bad := range []string{
+		"LOCK TABLE t IN FOO MODE",
+		"LOCK TABLE t IN ROW MODE",
+		"LOCK TABLE t IN MODE",
+		"LOCK TABLE t IN SHARE EXCLUSIVE MODE",
+		"LOCK TABLE t IN EXCLUSIVE ROW MODE",
+		"LOCK TABLE t IN ROW SHARE UPDATE MODE",
+		"LOCK TABLE t IN EXCLUSIVE MODE WAIT",
+		"LOCK TABLE t IN EXCLUSIVE MODE WAIT -1",
+		"LOCK TABLE t IN EXCLUSIVE MODE WAIT 2.5",
+		"LOCK TABLE t IN EXCLUSIVE MODE WAIT 1 + 1",
+		"LOCK TABLE t PARTITION () IN EXCLUSIVE MODE",
+		"LOCK TABLE t PARTITION (p1, p2) IN EXCLUSIVE MODE",
+		"LOCK TABLE t PARTITION FOR () IN EXCLUSIVE MODE",
+		"BEGIN LOCK TABLE t IN FOO MODE; END;",
+		"DECLARE c CONSTANT PLS_INTEGER := 5; BEGIN LOCK TABLE t IN EXCLUSIVE MODE WAIT c; END;",
+	} {
+		ParseShouldFail(t, bad)
+	}
+}

oracle/parser/parser.go

@@ -27,6 +27,11 @@ type Parser struct {
 	// met to the end of the range, whether it opens a CASE expression (true)
 	// or names a column (false). decideCaseTokens fills it in one pass.
 	caseKinds map[int]bool
+
+	// plsqlTypeMods is set while parsePLSQLTypeName parses the datatype of
+	// a PL/SQL declaration, where a length, precision, or scale may be an
+	// expression rather than an integer literal.
+	plsqlTypeMods bool
 }
 
 // Parse parses a SQL string into an AST list.
@@ -252,15 +257,7 @@ func (p *Parser) parseStmt() (nodes.StmtNode, error) {
 	case kwSET:
 		next := p.peekNext()
 		if next.Type == kwTRANSACTION {
-			start := p.pos()
-			p.advance() // consume SET
-			p.advance() // consume TRANSACTION
-			stmt, parseErr828 := p.parseSetTransactionStmt()
-			if parseErr828 != nil {
-				return nil, parseErr828
-			}
-			stmt.(*nodes.SetTransactionStmt).Loc.Start = start
-			return stmt, nil
+			return p.parseSetTransactionAtSet()
 		}
 		if next.Type == kwROLE {
 			return p.parseSetRoleStmt()