Split keywords into reserved and non-reserved sets

parser/keyword.go

@@ -259,6 +259,70 @@ const (
 	KeywordSecurity     = "SECURITY"
 )
 
+// reservedKeywords are the structural keywords — statement starters, clause
+// starters and expression operators — that cannot be used as bare identifiers:
+// letting them match identifier positions makes a missing name silently swallow
+// the next clause (e.g. `SELECT a FROM WHERE b = 1` parsing FROM as an alias).
+// Every other keyword is non-reserved and is accepted anywhere an identifier
+// is expected (see Parser.matchTokenKind). Positions where even a reserved
+// keyword is provably used as a name — after AS, after a dot in a qualified
+// name, or a lookahead-disambiguated select item — use parseAnyKeyword.
+//
+// Keywords that double as ClickHouse function or engine names (IF, LEFT,
+// RIGHT, ANY, MIN, MAX, TRIM, SET, JOIN...) must stay non-reserved.
+var reservedKeywords = NewSet(
+	KeywordAlter,
+	KeywordAnd,
+	KeywordAs,
+	KeywordBetween,
+	KeywordBy,
+	KeywordCase,
+	KeywordCreate,
+	KeywordCross,
+	KeywordDescribe,
+	KeywordDistinct,
+	KeywordDrop,
+	KeywordElse,
+	KeywordEnd,
+	KeywordExcept,
+	KeywordExplain,
+	KeywordFormat,
+	KeywordFrom,
+	KeywordGrant,
+	KeywordGroup,
+	KeywordHaving,
+	KeywordIlike,
+	KeywordIn,
+	KeywordInner,
+	KeywordInsert,
+	KeywordInterval,
+	KeywordInto,
+	KeywordIs,
+	KeywordKill,
+	KeywordLike,
+	KeywordLimit,
+	KeywordNot,
+	KeywordOffset,
+	KeywordOn,
+	KeywordOptimize,
+	KeywordOr,
+	KeywordOrder,
+	KeywordPrewhere,
+	KeywordRename,
+	KeywordSelect,
+	KeywordSettings,
+	KeywordShow,
+	KeywordThen,
+	KeywordTruncate,
+	KeywordUnion,
+	KeywordUse,
+	KeywordUsing,
+	KeywordWhen,
+	KeywordWhere,
+	KeywordWindow,
+	KeywordWith,
+)
+
 var keywords = NewSet(
 	KeywordAdd,
 	KeywordAdmin,

parser/keyword_test.go

@@ -0,0 +1,121 @@
+package parser
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestReservedKeywordsAreKeywords(t *testing.T) {
+	for _, kw := range reservedKeywords.Members() {
+		require.True(t, keywords.Contains(kw),
+			"reserved keyword %q is missing from the keywords set", kw)
+	}
+}
+
+// TestReservedKeywordRejectedAsIdentifier asserts that a reserved keyword no
+// longer silently fills an identifier slot: a missing name before a clause
+// keyword must fail at the keyword instead of swallowing it (the bug class
+// behind #268/#269: e.g. `SELECT a FROM WHERE b = 1` used to parse FROM as a
+// bare alias of `a`).
+func TestReservedKeywordRejectedAsIdentifier(t *testing.T) {
+	cases := []string{
+		"SELECT a FROM WHERE b = 1",                  // FROM must not become an alias of `a`
+		"SELECT a, b FROM GROUP BY a",                // FROM must not become an alias of `b`
+		"INSERT INTO SELECT 1",                       // SELECT must not become a table name
+		"SELECT a AS FROM t",                         // explicit alias FROM consumes the clause keyword
+		"SELECT a FROM t JOIN ON a = b",              // ON must not become a table name
+		"CREATE TABLE t (from String) ENGINE=Memory", // reserved keyword as column name needs quoting
+	}
+	for _, sql := range cases {
+		t.Run(sql, func(t *testing.T) {
+			_, err := NewParser(sql).ParseStmts()
+			require.Error(t, err)
+		})
+	}
+}
+
+// TestNonReservedKeywordAsIdentifier asserts that non-reserved keywords keep
+// working as identifiers anywhere an identifier is expected.
+func TestNonReservedKeywordAsIdentifier(t *testing.T) {
+	cases := []string{
+		"SELECT key FROM t",
+		"SELECT date, first, last, timestamp FROM t",
+		"CREATE TABLE t (key String, date Date) ENGINE=Memory",
+		"SELECT t.key FROM t",
+		"SELECT if(a, 1, 2), any(b), left(c, 1) FROM t",
+		"SELECT * FROM t WHERE key = 1",
+	}
+	for _, sql := range cases {
+		t.Run(sql, func(t *testing.T) {
+			_, err := NewParser(sql).ParseStmts()
+			require.NoError(t, err)
+		})
+	}
+}
+
+// TestReservedKeywordInDisambiguatedPositions asserts that reserved keywords
+// are still accepted as names where context proves they cannot start a clause:
+// after AS, after a dot in a qualified name, lookahead-disambiguated select
+// items, query parameters, window names, and GRANT options.
+func TestReservedKeywordInDisambiguatedPositions(t *testing.T) {
+	cases := []string{
+		"SELECT 1 AS from",
+		"SELECT 1 AS interval, 2 AS from, 3 AS limit",
+		"SELECT * FROM t AS from",
+		"SELECT a FROM db.from",
+		"SELECT t.from FROM t",
+		"SELECT a, limit FROM t",
+		"SELECT case;",
+		"SELECT limit",
+		"SELECT a FROM t WHERE ts < {end:UInt32}",
+		"SELECT sum(x) OVER (order) FROM t WINDOW order AS (PARTITION BY team)",
+		"SELECT sum(x) OVER order FROM t WINDOW order AS (PARTITION BY team)",
+		"GRANT SELECT(x) ON db.table TO john WITH GRANT OPTION",
+	}
+	for _, sql := range cases {
+		t.Run(sql, func(t *testing.T) {
+			_, err := NewParser(sql).ParseStmts()
+			require.NoError(t, err)
+		})
+	}
+}
+
+// TestReservedOperatorKeywordsAreCallable covers the regressions from the
+// review of #275: operator keywords double as ordinary ClickHouse function
+// names and must stay callable when followed by '('.
+func TestReservedOperatorKeywordsAreCallable(t *testing.T) {
+	inputs := []string{
+		"SELECT and(a, b) FROM t",
+		"SELECT or(a, b) FROM t",
+		"SELECT in(1, [1])",
+		"SELECT like(s, '%a%') FROM t",
+		"SELECT ilike(s, '%a%') FROM t",
+	}
+	for _, sql := range inputs {
+		t.Run(sql, func(t *testing.T) {
+			stmts, err := NewParser(sql).ParseStmts()
+			require.NoError(t, err)
+			require.Len(t, stmts, 1)
+		})
+	}
+}
+
+// TestReservedKeywordAliasesAfterAs covers the review regressions of #275:
+// AS proves the next token is an alias name, so even reserved keywords are
+// accepted in expression lists, ORDER BY, and non-parenthesized CTEs.
+func TestReservedKeywordAliasesAfterAs(t *testing.T) {
+	inputs := []string{
+		"SELECT (1 AS from)",
+		"SELECT sum(x AS from) FROM t",
+		"SELECT a FROM t ORDER BY x AS from",
+		"WITH 1 AS from SELECT from",
+	}
+	for _, sql := range inputs {
+		t.Run(sql, func(t *testing.T) {
+			stmts, err := NewParser(sql).ParseStmts()
+			require.NoError(t, err)
+			require.Len(t, stmts, 1)
+		})
+	}
+}

parser/parse_system.go

@@ -1278,7 +1278,9 @@ func (p *Parser) parseGrantOption(_ Pos) (string, error) {
 	if err := p.expectKeyword(KeywordWith); err != nil {
 		return "", err
 	}
-	ident, err := p.parseIdent()
+	// Between WITH and OPTION the token can only be the option name, which may
+	// be a reserved keyword (e.g. `WITH GRANT OPTION`).
+	ident, err := p.parseAnyKeyword()
 	if err != nil {
 		return "", err
 	}

parser/parser_column.go

@@ -152,8 +152,10 @@ func (p *Parser) parseInfix(expr Expr, precedence int) (Expr, error) {
 		// access column with dot notation
 		var rightExpr Expr
 		var err error
-		if p.matchTokenKind(TokenKindIdent) {
-			rightExpr, err = p.parseIdent()
+		if p.matchTokenKind(TokenKindIdent, TokenKindKeyword) {
+			// After a dot the token can only be a member name, so even
+			// reserved keywords are accepted (e.g. `t.from`).
+			rightExpr, err = p.parseAnyKeyword()
 		} else {
 			rightExpr, err = p.parseDecimal(p.Pos())
 		}
@@ -472,7 +474,7 @@ func (p *Parser) parseColumnExpr(pos Pos) (Expr, error) { //nolint:funlen
 	// terminator/alias check).
 	if p.keywordIsSelectItemIdentifier() ||
 		(p.matchTokenKind(TokenKindKeyword) && p.peekIsEndOfStatement()) {
-		return p.parseIdent()
+		return p.parseAnyKeyword()
 	}
 	switch {
 	case p.matchKeyword(KeywordInterval):
@@ -496,6 +498,12 @@ func (p *Parser) parseColumnExpr(pos Pos) (Expr, error) { //nolint:funlen
 		return p.parseColumnExtractExpr(pos)
 	case p.matchTokenKind(TokenKindIdent):
 		return p.parseIdentOrFunction(pos)
+	case p.currentTokenKind() == TokenKindKeyword && p.peekTokenKind(TokenKindLParen):
+		// Reserved operator keywords stay callable as ordinary functions when
+		// followed by '(': and(a, b), or(a, b), in(x, set), like(s, pat), ...
+		// Keywords with dedicated syntax (CAST, CASE, EXTRACT, INTERVAL, ...)
+		// are handled by their own cases above.
+		return p.parseIdentOrFunction(pos)
 	case p.matchTokenKind(TokenKindString): // string literal
 		return p.parseString(pos)
 	case p.matchTokenKind(TokenKindInt),
@@ -676,8 +684,10 @@ func (p *Parser) parseInterval(requireKeyword bool) (*IntervalExpr, error) {
 }
 
 func (p *Parser) parseFunctionExpr(_ Pos) (*FunctionExpr, error) {
-	// parse function name
-	name, err := p.parseIdent()
+	// parse function name; callers gate entry (select-item modifiers match
+	// EXCEPT/APPLY/REPLACE first, INSERT INTO FUNCTION follows the FUNCTION
+	// keyword), so even reserved keywords are valid names here.
+	name, err := p.parseAnyKeyword()
 	if err != nil {
 		return nil, err
 	}
@@ -794,7 +804,9 @@ func (p *Parser) parseQueryParam(pos Pos) (*QueryParam, error) {
 		return nil, err
 	}
 
-	ident, err := p.parseIdent()
+	// Inside `{name:Type}` the token can only be the parameter name, so even
+	// reserved keywords are accepted (e.g. `{end:UInt32}`).
+	ident, err := p.parseAnyKeyword()
 	if err != nil {
 		return nil, err
 	}
@@ -844,7 +856,8 @@ func (p *Parser) parseColumnsExpr(pos Pos) (*ColumnExpr, error) {
 
 	var alias *Ident
 	if p.tryConsumeKeywords(KeywordAs) {
-		alias, err = p.parseIdent()
+		// after AS the token can only be an alias name, reserved keyword or not
+		alias, err = p.parseAnyKeyword()
 		if err != nil {
 			return nil, err
 		}
@@ -877,15 +890,16 @@ func (p *Parser) parseSelectItem() (*SelectItem, error) {
 	var alias *Ident
 	switch {
 	case p.tryConsumeKeywords(KeywordAs):
-		// `SELECT 1 AS <reserved-keyword>` works for any keyword because
-		// matchTokenKind(TokenKindIdent) coerces TokenKindKeyword to ident
-		// (see parser_common.go matchTokenKind), so parseIdent accepts a
-		// keyword token here without needing a special-case.
-		alias, err = p.parseIdent()
+		// `SELECT 1 AS <keyword>` works for any keyword, reserved or not:
+		// after AS the token can only be an alias name.
+		alias, err = p.parseAnyKeyword()
 		if err != nil {
 			return nil, err
 		}
-	case p.currentTokenKind() == TokenKindKeyword && !p.isSelectItemTerminatorKeyword():
+	case p.currentTokenKind() == TokenKindKeyword &&
+		p.matchTokenKind(TokenKindIdent) && !p.isSelectItemTerminatorKeyword():
+		// Only a non-reserved keyword can be a bare (no-AS) alias; a reserved
+		// keyword here starts the next clause (e.g. `SELECT a FROM ...`).
 		alias, err = p.parseIdent()
 		if err != nil {
 			return nil, err

parser/parser_common.go

@@ -64,9 +64,30 @@ func (p *Parser) Pos() Pos {
 	return last.Pos
 }
 
-func (p *Parser) matchTokenKind(kind TokenKind) bool {
-	return p.currentTokenKind() == kind ||
-		(kind == TokenKindIdent && p.currentTokenKind() == TokenKindKeyword)
+// matchTokenKind reports whether the current token matches any of the given
+// kinds. A non-reserved keyword also matches TokenKindIdent: most ClickHouse
+// keywords (DATE, KEY, FIRST, ...) are valid identifiers anywhere an
+// identifier is expected. Reserved keywords (see reservedKeywords) do not
+// match TokenKindIdent, so a missing identifier before e.g. FROM or WHERE
+// fails fast instead of silently swallowing the clause keyword as a name; use
+// matchTokenKind(TokenKindIdent, TokenKindKeyword) where any keyword -
+// reserved or not - is acceptable.
+func (p *Parser) matchTokenKind(kinds ...TokenKind) bool {
+	cur := p.currentTokenKind()
+	for _, kind := range kinds {
+		if cur == kind {
+			return true
+		}
+	}
+	if cur != TokenKindKeyword {
+		return false
+	}
+	for _, kind := range kinds {
+		if kind == TokenKindIdent {
+			return !reservedKeywords.Contains(strings.ToUpper(p.current().String))
+		}
+	}
+	return false
 }
 
 // expectTokenKind consumes the current token if it is the given kind.
@@ -141,6 +162,29 @@ func (p *Parser) tryParseIdent() *Ident {
 	}
 }
 
+// parseAnyKeyword parses the current token as an identifier, accepting
+// any keyword token — reserved or not — as the name. Use it only in positions
+// where context has already proven the token is a name and not the start of a
+// clause or expression: after AS, after a dot in a qualified name, or a select
+// item the lookahead disambiguated.
+func (p *Parser) parseAnyKeyword() (*Ident, error) {
+	last := p.current()
+	if !p.matchTokenKind(TokenKindIdent, TokenKindKeyword) {
+		return nil, &ParseError{
+			Pos:      p.Pos(),
+			Got:      last,
+			Expected: []TokenKind{TokenKindIdent},
+		}
+	}
+	_ = p.lexer.consumeToken()
+	return &Ident{
+		NamePos:   last.Pos,
+		NameEnd:   last.End,
+		Name:      last.String,
+		QuoteType: last.QuoteType,
+	}, nil
+}
+
 func (p *Parser) parseIdent() (*Ident, error) {
 	curToken := p.current()
 	if err := p.expectTokenKind(TokenKindIdent); err != nil {
@@ -194,13 +238,20 @@ func (p *Parser) tryParseDotIdent(_ Pos) (*Ident, error) {
 	if p.tryConsumeTokenKind(TokenKindDot) == nil {
 		return nil, nil // nolint
 	}
-	return p.parseIdent()
+	// After a dot the token can only be a member name, so even reserved
+	// keywords are accepted (e.g. `db.from`, `t.limit`).
+	return p.parseAnyKeyword()
 }
 
 func (p *Parser) tryParseDotIdentOrString(_ Pos) (*Ident, error) {
 	if p.tryConsumeTokenKind(TokenKindDot) == nil {
 		return nil, nil // nolint
 	}
+	// After a dot the token can only be a member name, so even reserved
+	// keywords are accepted (e.g. `db.from`).
+	if p.currentTokenKind() == TokenKindKeyword {
+		return p.parseAnyKeyword()
+	}
 	return p.parseIdentOrString()
 }