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.

Sign up for GitHub

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

Fix parser fallback to LL mode #18852

Merged
merged 3 commits into from
Aug 30, 2023
Merged

Fix parser fallback to LL mode #18852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e4c17d
Fix parser fallback to LL mode
electrum Aug 29, 2023
3ba6e94
Fix exception for StackOverflow in SparkExpressionParser
electrum Aug 29, 2023
a80adea
Cleanup warnings in parsers
electrum Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 8 additions & 29 deletions core/trino-parser/src/main/java/io/trino/sql/jsonpath/PathParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.atn.PredictionMode;
import org.antlr.v4.runtime.misc.Pair;
import org.antlr.v4.runtime.misc.ParseCancellationException;
import org.antlr.v4.runtime.tree.TerminalNode;

import java.util.Arrays;
import java.util.List;

import static com.google.common.base.Preconditions.checkArgument;
import static java.util.Objects.requireNonNull;

public final class PathParser
Expand All @@ -44,8 +44,8 @@ public PathParser(Location startLocation)
{
requireNonNull(startLocation, "startLocation is null");

int pathStartLine = startLocation.line;
int pathStartColumn = startLocation.column;
int pathStartLine = startLocation.line();
int pathStartColumn = startLocation.column();
this.errorListener = new BaseErrorListener()
{
@Override
Expand Down Expand Up @@ -83,7 +83,7 @@ public PathNode parseJsonPath(String path)
parser.getInterpreter().setPredictionMode(PredictionMode.SLL);
tree = parser.path();
}
catch (ParseCancellationException ex) {
catch (ParsingException ex) {
// if we fail, parse with LL mode
tokenStream.seek(0); // rewind input stream
parser.reset();
Expand Down Expand Up @@ -135,33 +135,12 @@ public void exitNonReserved(JsonPathParser.NonReservedContext context)
}
}

public static class Location
public record Location(int line, int column)
{
private final int line;
private final int column;

public Location(int line, int column)
{
if (line < 1) {
throw new IllegalArgumentException("line must be at least 1");
}

if (column < 0) {
throw new IllegalArgumentException("column must be at least 0");
}

this.line = line;
this.column = column;
}

public int getLine()
{
return line;
}

public int getColumn()
public Location
{
return column;
checkArgument(line >= 1, "line must be at least 1");
checkArgument(column >= 0, "column must be at least 0");
}
}
}
26 changes: 14 additions & 12 deletions core/trino-parser/src/main/java/io/trino/sql/parser/SqlParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.trino.sql.tree.PathSpecification;
import io.trino.sql.tree.RowPattern;
import io.trino.sql.tree.Statement;
import org.antlr.v4.runtime.ANTLRErrorListener;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CommonToken;
Expand All @@ -36,7 +37,6 @@
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.atn.PredictionMode;
import org.antlr.v4.runtime.misc.Pair;
import org.antlr.v4.runtime.misc.ParseCancellationException;
import org.antlr.v4.runtime.tree.TerminalNode;

import java.util.Arrays;
Expand All @@ -49,7 +49,7 @@

public class SqlParser
{
private static final BaseErrorListener LEXER_ERROR_LISTENER = new BaseErrorListener()
private static final ANTLRErrorListener LEXER_ERROR_LISTENER = new BaseErrorListener()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the interface needed by ANTLR. The base class is just a convenience for implementations. It’s like declaring as Map rather than AbstractMap.

{
@Override
public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int line, int charPositionInLine, String message, RecognitionException e)
Expand Down Expand Up @@ -152,17 +152,19 @@ public Token recoverInline(Parser recognizer)

ParserRuleContext tree;
try {
// first, try parsing with potentially faster SLL mode
parser.getInterpreter().setPredictionMode(PredictionMode.SLL);
tree = parseFunction.apply(parser);
}
catch (ParseCancellationException ex) {
// if we fail, parse with LL mode
tokenStream.seek(0); // rewind input stream
parser.reset();
try {
// first, try parsing with potentially faster SLL mode
parser.getInterpreter().setPredictionMode(PredictionMode.SLL);
tree = parseFunction.apply(parser);
}
catch (ParsingException ex) {
// if we fail, parse with LL mode
tokenStream.seek(0); // rewind input stream
parser.reset();

parser.getInterpreter().setPredictionMode(PredictionMode.LL);
tree = parseFunction.apply(parser);
parser.getInterpreter().setPredictionMode(PredictionMode.LL);
tree = parseFunction.apply(parser);
}
}
catch (ParsingException e) {
location.ifPresent(statementLocation -> {
Expand Down
34 changes: 3 additions & 31 deletions core/trino-parser/src/main/java/io/trino/type/TypeCalculation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@
import io.trino.grammar.type.TypeCalculationParser.ParenthesizedExpressionContext;
import io.trino.grammar.type.TypeCalculationParser.TypeCalculationContext;
import io.trino.sql.parser.ParsingException;
import org.antlr.v4.runtime.ANTLRErrorListener;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.RecognitionException;
import org.antlr.v4.runtime.Recognizer;
import org.antlr.v4.runtime.atn.PredictionMode;
import org.antlr.v4.runtime.misc.ParseCancellationException;

import java.math.BigInteger;
import java.util.Map;
Expand All @@ -48,7 +48,7 @@

public final class TypeCalculation
{
private static final BaseErrorListener ERROR_LISTENER = new BaseErrorListener()
private static final ANTLRErrorListener ERROR_LISTENER = new BaseErrorListener()
{
@Override
public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int line, int charPositionInLine, String message, RecognitionException e)
Expand Down Expand Up @@ -92,7 +92,7 @@ private static ParserRuleContext parseTypeCalculation(String calculation)
parser.getInterpreter().setPredictionMode(PredictionMode.SLL);
tree = parser.typeCalculation();
}
catch (ParseCancellationException ex) {
catch (ParsingException ex) {
// if we fail, parse with LL mode
tokenStream.seek(0); // rewind input stream
parser.reset();
Expand All @@ -103,34 +103,6 @@ private static ParserRuleContext parseTypeCalculation(String calculation)
return tree;
}

private static class IsSimpleExpressionVisitor
extends TypeCalculationBaseVisitor<Boolean>
{
@Override
public Boolean visitArithmeticBinary(ArithmeticBinaryContext ctx)
{
return false;
}

@Override
public Boolean visitArithmeticUnary(ArithmeticUnaryContext ctx)
{
return false;
}

@Override
protected Boolean defaultResult()
{
return true;
}

@Override
protected Boolean aggregateResult(Boolean aggregate, Boolean nextResult)
{
return aggregate && nextResult;
}
}

private static class CalculateTypeVisitor
extends TypeCalculationBaseVisitor<BigInteger>
{
Expand Down
6 changes: 3 additions & 3 deletions ...rino-base-jdbc/src/main/java/io/trino/plugin/jdbc/expression/ExpressionMappingParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
package io.trino.plugin.jdbc.expression;

import com.google.common.collect.ImmutableMap;
import org.antlr.v4.runtime.ANTLRErrorListener;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.RecognitionException;
import org.antlr.v4.runtime.Recognizer;
import org.antlr.v4.runtime.atn.PredictionMode;
import org.antlr.v4.runtime.misc.ParseCancellationException;

import java.util.Map;
import java.util.Set;
Expand All @@ -32,7 +32,7 @@

public class ExpressionMappingParser
{
private static final BaseErrorListener ERROR_LISTENER = new BaseErrorListener()
private static final ANTLRErrorListener ERROR_LISTENER = new BaseErrorListener()
{
@Override
public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int line, int charPositionInLine, String message, RecognitionException e)
Expand Down Expand Up @@ -77,7 +77,7 @@ public Object invokeParser(String input, Function<ConnectorExpressionPatternPars
parser.getInterpreter().setPredictionMode(PredictionMode.SLL);
tree = parseFunction.apply(parser);
}
catch (ParseCancellationException ex) {
catch (IllegalArgumentException ex) {
// if we fail, parse with LL mode
tokenStream.seek(0); // rewind input stream
parser.reset();
Expand Down
8 changes: 4 additions & 4 deletions ...-delta-lake/src/main/java/io/trino/plugin/deltalake/expression/SparkExpressionParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,22 @@
package io.trino.plugin.deltalake.expression;

import com.google.common.annotations.VisibleForTesting;
import org.antlr.v4.runtime.ANTLRErrorListener;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.RecognitionException;
import org.antlr.v4.runtime.Recognizer;
import org.antlr.v4.runtime.atn.PredictionMode;
import org.antlr.v4.runtime.misc.ParseCancellationException;

import java.util.function.Function;

import static com.google.common.base.MoreObjects.firstNonNull;

public final class SparkExpressionParser
{
private static final BaseErrorListener ERROR_LISTENER = new BaseErrorListener()
private static final ANTLRErrorListener ERROR_LISTENER = new BaseErrorListener()
{
@Override
public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int line, int charPositionInLine, String message, RecognitionException e)
Expand Down Expand Up @@ -76,7 +76,7 @@ private static Object invokeParser(String input, Function<SparkExpressionBasePar
parser.getInterpreter().setPredictionMode(PredictionMode.SLL);
tree = parseFunction.apply(parser);
}
catch (ParseCancellationException ex) {
catch (ParsingException ex) {
// if we fail, parse with LL mode
tokenStream.seek(0); // rewind input stream
parser.reset();
Expand All @@ -87,7 +87,7 @@ private static Object invokeParser(String input, Function<SparkExpressionBasePar
return new SparkExpressionBuilder().visit(tree);
}
catch (StackOverflowError e) {
throw new IllegalArgumentException("expression is too large (stack overflow while parsing)");
throw new ParsingException("expression is too large (stack overflow while parsing)");
}
}
}