Unable to parse Trino statement with non-escaped limit column

[john-bodley]

According to the documentation LIMIT is not a reserved keyword, though it seems like SQLGlot is unable to parse statements referring to a column named limit unless it is escaped.

Before you file an issue

• Make sure you specify the "read" dialect eg. parse_one(sql, read="spark")
• Make sure you specify the "write" dialect eg. ast.sql(dialect="duckdb")
• Check if the issue still exists on main

Fully reproducible code snippet
Please include a fully reproducible code snippet or the input sql, dialect, and expected output.

>>> parse_one("SELECT limit FROM foo.bar", read="trino")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/__init__.py", line 124, in parse_one
    result = dialect.parse(sql, **opts)
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/dialects/dialect.py", line 490, in parse
    return self.parser(**opts).parse(self.tokenize(sql), sql)
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 1163, in parse
    return self._parse(
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 1229, in _parse
    expressions.append(parse_method(self))
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 1458, in _parse_statement
    expression = self._parse_set_operations(expression) if expression else self._parse_select()
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 2521, in _parse_select
    this = self._parse_query_modifiers(this)
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 2669, in _parse_query_modifiers
    key, expression = parser(self)
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 952, in <lambda>
    TokenType.LIMIT: lambda self: ("limit", self._parse_limit()),
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 3620, in _parse_limit
    limit_exp = self.expression(
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 1293, in expression
    return self.validate_expression(instance)
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 1313, in validate_expression
    self.raise_error(error_message)
  File "/Users/johnbodley/repos/github.com/sqlglot/sqlglot/parser.py", line 1273, in raise_error
    raise error
sqlglot.errors.ParseError: Required keyword: 'expression' missing for <class 'sqlglot.expressions.Limit'>. Line 1, Col: 17.
  SELECT limit FROM foo.bar

Note that SQLGlot is able to parse the following statement if the limit column is escaped.

>>> parse_one('SELECT "limit" FROM foo.bar', read="trino")
Select(
  expressions=[
    Column(
      this=Identifier(this=limit, quoted=True))],
  from=From(
    this=Table(
      this=Identifier(this=bar, quoted=False),
      db=Identifier(this=foo, quoted=False))))

Official Documentation

• https://trino.io/docs/current/language/reserved.html

[tobymao]

this is a big pain to deal with. considering most other engines limit is a reserved keyword, i'm not inclined to make this change. it breaks a lot of other things, so closing this as won't fix.

[barakalon]

Don't we just need to add LIMIT to the trino parser's ID_VAR_TOKENS?

[barakalon]

from sqlglot import parse_one
from sqlglot.dialects.trino import Trino
from sqlglot.tokens import TokenType

Trino.Parser.ID_VAR_TOKENS.add(TokenType.LIMIT)

q = """
SELECT limit FROM limit ORDER BY limit LIMIT 1
"""

print(repr(parse_one(q, dialect="trino")))

Select(
  expressions=[
    Column(
      this=Identifier(this=limit, quoted=False))],
  limit=Limit(
    expression=Literal(this=1, is_string=False)),
  from=From(
    this=Table(
      this=Identifier(this=limit, quoted=False))),
  order=Order(
    expressions=[
      Ordered(
        this=Column(
          this=Identifier(this=limit, quoted=False)),
        nulls_first=False)]))

[georgesittas]

It's not that simple. I think your example works by accident and may have actually uncovered an interesting bug.

Basically, Dialect gets created before any of its subclasses, so TABLE_ALIAS_TOKENS is computed at that time as a function of Dialect.ID_VAR_TOKENS and it doesn't contain TokenType.LIMIT. This means that SELECT * FROM t LIMIT 1 correctly produces a Limit node instead of treating LIMIT as an alias of t.

Even if you add TokenType.LIMIT to a dialect's ID_VAR_TOKENS, it doesn't affect the TABLE_ALIAS_TOKENS of that dialect, so you still have the above behavior (the "accident"). I think the right approach would be to compute class attributes which depend on other class attributes in the metaclass to avoid this situation.

Using the above logic you can easily construct an example where Trino's parser would fail:

>>> from sqlglot import parse_one
>>> from sqlglot.dialects.trino import Trino
>>> from sqlglot.tokens import TokenType
>>>
>>> Trino.Parser.ID_VAR_TOKENS.add(TokenType.LIMIT)
>>>
>>> q = """
... SELECT a LIMIT 1
... """
>>>
>>> print(repr(parse_one(q, dialect="trino")))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "sqlglot/sqlglot/__init__.py", line 124, in parse_one
    result = dialect.parse(sql, **opts)
  File "sqlglot/sqlglot/dialects/dialect.py", line 490, in parse
    return self.parser(**opts).parse(self.tokenize(sql), sql)
  File "sqlglot/sqlglot/parser.py", line 1167, in parse
    return self._parse(
  File "sqlglot/sqlglot/parser.py", line 1236, in _parse
    self.raise_error("Invalid expression / Unexpected token")
  File "sqlglot/sqlglot/parser.py", line 1277, in raise_error
    raise error
sqlglot.errors.ParseError: Invalid expression / Unexpected token. Line 2, Col: 16.

SELECT a LIMIT 1
               ~

[tobymao]

don’t a ton of tests break when you add limit to id var?

[georgesittas]

Yes, if you add it in the base parser. If you add it to a dialect that doesn't override TABLE_ALIAS_TOKENS though, the same tests won't break for it (see above).

[tobymao]

i added limit to table alias tokens in base as well and all the tests broke.

[barakalon]

Ah I see.

FWIW, SELECT a LIMIT 1 isn't a valid query.

Here's trino's grammar.

Some tests on trino:

-- fails
SELECT 1 limit
-- succeeds
SELECT 1 AS limit

-- fails
SELECT limit FROM (SELECT 1 AS limit)
-- succeeds
SELECT "limit" FROM (SELECT 1 AS limit)

@john-bodley where did this come up? Does SELECT limit FROM foo.bar actually parse?

[john-bodley]

@barakalon this toy example is from Superset performing a SELECT *-esque query where it enumerates all the columns (as opposed to using the wildcard operator).

Regardless, SELECT limit FROM foo.bar is a valid query in Trino, i.e., one doesn’t need to explicitly escape the limit column.

[barakalon]

Based on SELECT limit FROM (SELECT 1 AS limit) failing to parse in trino, I'm curious how SELECT limit FROM foo.bar could be valid?
Give me a bit to test with a table that has a limit column, but I'm curious if you've actually tested such a query?

[barakalon]

oh - ha!
I'm testing on superset, so I think its actually sqlglot that's failing 🤦

[georgesittas]

oh - ha! I'm testing on superset, so I think its actually sqlglot that's failing 🤦

Lol