Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LIMIT statement is not correctly caught by the current regular expression #5272

Closed
conglei opened this issue Jun 22, 2018 · 1 comment · Fixed by #5295
Closed

LIMIT statement is not correctly caught by the current regular expression #5272

conglei opened this issue Jun 22, 2018 · 1 comment · Fixed by #5295

Comments

@conglei
Copy link
Contributor

conglei commented Jun 22, 2018

The current logic of catching LIMIT statement has an assumption that it will be always at the very end of a sql query. Due to this, the table content preview is broken on sqlite, since LIMIT 100 OFFSET 0 will be attached (from SqlAlchemy library). It results an extra LIMIT 1000000 being added and cause the fatal error.

The current RE:

limit_pattern = re.compile(r"""
	(?ix)          # case insensitive, verbose
	\s+            # whitespace
	LIMIT\s+(\d+)  # LIMIT $ROWS
	;?             # optional semi-colon
	(\s|;)*$       # remove trailing spaces tabs or semicolons
	""")

The possible solution:

limit_pattern = re.compile(r"""
	(?ix)          # case insensitive, verbose
	\s+            # whitespace
	LIMIT\s+(\d+)  # LIMIT $ROWS
	(\s|;|.)*$     # remove the rest
	""")

However, still, it cannot correctly capture all the cases.

Any suggestion is welcome!

@conglei conglei changed the title LIMIT statement is not correctly caught by current regular expression LIMIT statement is not correctly caught by the current regular expression Jun 22, 2018
@john-bodley
Copy link
Member

Note SQLAlchemy enforces an offset for SQLite whenever a limit is present as described in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants