prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
16.05k stars 5.38k forks source link

Prepared statement parse error with limit ? #14269

Open octoben opened 4 years ago

octoben commented 4 years ago

Using presto 0.214. Trying to create a prepared statement with limit ? and I get the following:

PREPARE statement1 FROM select ....
from .... 
limit ?
com.facebook.presto.sql.parser.ParsingException: line 3:7: mismatched input '?'. Expecting: 'ALL', <integer>
    at com.facebook.presto.sql.parser.ErrorHandler.syntaxError(ErrorHandler.java:109)
    at org.antlr.v4.runtime.ProxyErrorListener.syntaxError(ProxyErrorListener.java:41)
    at org.antlr.v4.runtime.Parser.notifyErrorListeners(Parser.java:544)
    at org.antlr.v4.runtime.DefaultErrorStrategy.reportInputMismatch(DefaultErrorStrategy.java:327)
    at org.antlr.v4.runtime.DefaultErrorStrategy.reportError(DefaultErrorStrategy.java:139)
    at com.facebook.presto.sql.parser.SqlBaseParser.queryNoWith(SqlBaseParser.java:3393)
    at com.facebook.presto.sql.parser.SqlBaseParser.query(SqlBaseParser.java:2751)
    at com.facebook.presto.sql.parser.SqlBaseParser.statement(SqlBaseParser.java:1453)
    at com.facebook.presto.sql.parser.SqlBaseParser.statement(SqlBaseParser.java:2605)
    at com.facebook.presto.sql.parser.SqlBaseParser.singleStatement(SqlBaseParser.java:236)
    at com.facebook.presto.sql.parser.SqlParser.invokeParser(SqlParser.java:160)
    at com.facebook.presto.sql.parser.SqlParser.createStatement(SqlParser.java:96)
    at com.facebook.presto.execution.QueryPreparer.prepareQuery(QueryPreparer.java:55)
    at com.facebook.presto.execution.SqlQueryManager.createQueryInternal(SqlQueryManager.java:338)
    at com.facebook.presto.execution.SqlQueryManager.lambda$createQuery$4(SqlQueryManager.java:303)
    at com.facebook.presto.$gen.Presto_0_214____20200311_120255_1.run(Unknown Source)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
Caused by: org.antlr.v4.runtime.InputMismatchException: undefined
    at com.facebook.presto.sql.parser.SqlParser$2.recoverInline(SqlParser.java:134)
    at com.facebook.presto.sql.parser.SqlBaseParser.queryNoWith(SqlBaseParser.java:3379)
    ... 15 more
etzelm commented 4 years ago

We also ran into this stack-trace when working on a project migration to Presto. I tried looking through the Presto source code to try to understand what happens and here is what I found:

I'm not really sure what the fix for this would be. It could be the obvious fix of adding '?' to the limit lexer rule, but I don't want to break other intended functionality I'm not aware of. The other option could be to modify the constructor's execute to better handle the dynamic '?' in a better way.

Either way having the dynamic functionality of using parameters in Prepared Statements is an extreme nice to have for us.

(P.S.: Does anyone know where Presto stores its ANTLR lexer rules? Couldn't find them...)

Update: Made a merge request using the first idea for a fix, should be mentioned below. I also found the lexer rules here.

etzelm commented 4 years ago

Seems like this issue is baked deeper into the Presto SQL Parsing than I originally thought. Even with the changes to the ANTLR4 Lexer rule, the main Presto SQL parsing validation is throwing errors because the '?' after LIMIT is not getting counted by the tree traversals as a parameter to be replaced. Here is the stack trace being seen by my modified unit tests:

[ERROR] Tests run: 112, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 97.098 s <<< FAILURE! - in TestSuite [ERROR] testExecuteQuery(com.facebook.presto.jdbc.TestJdbcPreparedStatement) Time elapsed: 0.081 s <<< FAILURE! java.sql.SQLException: Query failed (#20200709_081011_01157_3cv8w): line 1:1: Incorrect number of parameters: expected 1 but found 2 at com.facebook.presto.jdbc.PrestoResultSet.resultsException(PrestoResultSet.java:1840) at com.facebook.presto.jdbc.PrestoResultSet.getColumns(PrestoResultSet.java:1750) at com.facebook.presto.jdbc.PrestoResultSet.<init>(PrestoResultSet.java:119) at com.facebook.presto.jdbc.PrestoStatement.internalExecute(PrestoStatement.java:250) at com.facebook.presto.jdbc.PrestoStatement.execute(PrestoStatement.java:228) at com.facebook.presto.jdbc.PrestoPreparedStatement.executeQuery(PrestoPreparedStatement.java:99) at com.facebook.presto.jdbc.TestJdbcPreparedStatement.testExecuteQuery(TestJdbcPreparedStatement.java:88) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:104) at org.testng.internal.Invoker.invokeMethod(Invoker.java:645) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:851) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1177) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at java.lang.Thread.run(Thread.java:748) Caused by: com.facebook.presto.sql.analyzer.SemanticException: line 1:1: Incorrect number of parameters: expected 1 but found 2 at com.facebook.presto.execution.QueryPreparer.validateParameters(QueryPreparer.java:105) at com.facebook.presto.execution.QueryPreparer.prepareQuery(QueryPreparer.java:87) at com.facebook.presto.execution.QueryPreparer.prepareQuery(QueryPreparer.java:69) at com.facebook.presto.dispatcher.DispatchManager.createQueryInternal(DispatchManager.java:183) at com.facebook.presto.dispatcher.DispatchManager.lambda$createQuery$0(DispatchManager.java:154) at com.facebook.airlift.concurrent.BoundedExecutor.drainQueue(BoundedExecutor.java:78) ... 3 more

I'll try to continue work on this when I find some free time.

octoben commented 4 years ago

@etzelm We use QueryDSL as an "ORM" for our application. To temporarily get around this issue I wrote a template that uses literals for limit and offset. It was a little bit of a hack but it's working now. Depending on how you create your queries you may or not be able to do the same.

etzelm commented 4 years ago

Yes, we ended up doing a similar work around for now but I'd still like to see this eventually get fixed as well because according to the documentation it should work. Thank you :)

etzelm commented 4 years ago

Heads up, Martin Traverso(Co-Creator of Presto) followed up with me and it looks like this was fixed in the latest build here: https://prestosql.io/docs/current/release/release-340.html

nayendur commented 3 years ago

Any plans to address this issue from presto db side a part from the issue being fixed at presto-sql which is now Trino ?