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

[#857] Fix some regressions in JPQLNext parser #862

Merged
merged 2 commits into from
Oct 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public Predicate createBooleanExpression(final String expression, final boolean

private <E extends Expression> E getOrDefault(String cacheName, ExpressionFactory expressionFactory, String expression, boolean allowOuter, boolean allowQuantifiedPredicates, boolean allowObjectExpression, MacroConfiguration macroConfiguration, ExpressionSupplier defaultExpressionSupplier) {
// Find the expression cache entry
ExpressionCacheEntry exprEntry = expressionCache.get(cacheName, expression);
ExpressionCacheEntry exprEntry = expressionCache.get(cacheName, new ExpressionCache.Key(expression, allowOuter, allowQuantifiedPredicates, allowObjectExpression));
MacroConfiguration macroKey = null;
Expression expr;

Expand All @@ -168,7 +168,7 @@ private <E extends Expression> E getOrDefault(String cacheName, ExpressionFactor
exprEntry.addMacroConfigurationExpression(macroKey, expr);
}

expressionCache.putIfAbsent(cacheName, expression, exprEntry);
expressionCache.putIfAbsent(cacheName, new ExpressionCache.Key(expression, allowOuter, allowQuantifiedPredicates, allowObjectExpression), exprEntry);
return (E) expr.clone(false);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.blazebit.persistence.parser.JPQLNextParser;
import com.blazebit.persistence.parser.predicate.Predicate;
import org.antlr.v4.runtime.ANTLRErrorListener;
import org.antlr.v4.runtime.CharStream;
import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.Parser;
Expand Down Expand Up @@ -157,7 +158,8 @@ private Expression createExpression(RuleInvoker ruleInvoker, String expression,
if (expression.isEmpty()) {
throw new IllegalArgumentException("expression");
}
JPQLNextLexer l = new JPQLNextLexer(CharStreams.fromString(expression));
CharStream inputCharStream = CharStreams.fromString(expression);
JPQLNextLexer l = new JPQLNextLexer(inputCharStream);
configureLexer(l);
CommonTokenStream tokens = new CommonTokenStream(l);
JPQLNextParser p = new JPQLNextParser(tokens);
Expand All @@ -182,7 +184,7 @@ private Expression createExpression(RuleInvoker ruleInvoker, String expression,
LOG.finest(ctx.toStringTree());
}

JPQLNextExpressionVisitorImpl visitor = new JPQLNextExpressionVisitorImpl(functions, enumTypes, entityTypes, minEnumSegmentCount, minEntitySegmentCount, macroConfiguration == null ? Collections.EMPTY_MAP : macroConfiguration.macros, usedMacros, allowOuter, allowQuantifiedPredicates, allowObjectExpression);
JPQLNextExpressionVisitorImpl visitor = new JPQLNextExpressionVisitorImpl(functions, enumTypes, entityTypes, minEnumSegmentCount, minEntitySegmentCount, macroConfiguration == null ? Collections.EMPTY_MAP : macroConfiguration.macros, usedMacros, allowOuter, allowQuantifiedPredicates, allowObjectExpression, inputCharStream);
Expression parsedExpression = visitor.visit(ctx);
if (optimize) {
parsedExpression = parsedExpression.accept(optimizer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,33 +26,33 @@
*/
public class ConcurrentHashMapExpressionCache<T> implements ExpressionCache<T> {

private final ConcurrentMap<String, ConcurrentMap<String, T>> cacheManager;
private final ConcurrentMap<String, ConcurrentMap<Key, T>> cacheManager;

public ConcurrentHashMapExpressionCache() {
this.cacheManager = new ConcurrentHashMap<>();
}

@Override
public T get(String cacheName, String expression) {
final ConcurrentMap<String, T> cache = cacheManager.get(cacheName);
return cache == null ? null : cache.get(expression);
public T get(String cacheName, Key key) {
final ConcurrentMap<Key, T> cache = cacheManager.get(cacheName);
return cache == null ? null : cache.get(key);
}

@Override
public T putIfAbsent(String cacheName, String expression, T value) {
public T putIfAbsent(String cacheName, Key key, T value) {
// Find the cache manager
ConcurrentMap<String, T> cache = cacheManager.get(cacheName);
ConcurrentMap<Key, T> cache = cacheManager.get(cacheName);

if (cache == null) {
cache = new ConcurrentHashMap<>();
ConcurrentMap<String, T> oldCache = cacheManager.putIfAbsent(cacheName, cache);
ConcurrentMap<Key, T> oldCache = cacheManager.putIfAbsent(cacheName, cache);

if (oldCache != null) {
cache = oldCache;
}
}

T oldValue = cache.putIfAbsent(expression, value);
T oldValue = cache.putIfAbsent(key, value);
if (oldValue != null) {
return oldValue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,76 @@

package com.blazebit.persistence.parser.expression;

import java.util.Objects;

/**
*
* @author Christian Beikov
* @since 1.2.0
*/
public interface ExpressionCache<T> {

public T get(String cacheName, String expression);
public T get(String cacheName, Key key);

public T putIfAbsent(String cacheName, Key key, T value);

/**
*
* @author Moritz Becker
* @since 1.4.0
*/
class Key {
private static final byte ALLOW_OUTER_MASK = 1;
private static final byte ALLOW_QUANTIFIED_PREDICATES_MASK = (1 << 1);
private static final byte ALLOW_OBJECT_EXPRESSION_MASK = (1 << 2);

private final String expression;
private final byte flags;

public Key(String expression, boolean allowOuter, boolean allowQuantifiedPredicates, boolean allowObjectExpression) {
this.expression = expression;
byte flags = 0;
if (allowOuter) {
flags |= ALLOW_OUTER_MASK;
}
if (allowOuter) {
flags |= ALLOW_QUANTIFIED_PREDICATES_MASK;
}
if (allowObjectExpression) {
flags |= ALLOW_OBJECT_EXPRESSION_MASK;
}
this.flags = flags;
}

public boolean isAllowOuter() {
return (flags & ALLOW_OUTER_MASK) != 0;
}

public boolean isAllowQuantifiedPredicates() {
return (flags & ALLOW_QUANTIFIED_PREDICATES_MASK) != 0;
}

public boolean isAllowObjectExpression() {
return (flags & ALLOW_OBJECT_EXPRESSION_MASK) != 0;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
Key key = (Key) o;
return flags == key.flags &&
expression.equals(key.expression);
}

public T putIfAbsent(String cacheName, String expression, T value);
@Override
public int hashCode() {
return Objects.hash(expression, flags);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@
import com.blazebit.persistence.parser.predicate.Predicate;
import com.blazebit.persistence.parser.predicate.PredicateQuantifier;
import com.blazebit.persistence.parser.util.TypeUtils;
import org.antlr.v4.runtime.CharStream;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.misc.Interval;
import org.antlr.v4.runtime.tree.ErrorNode;
import org.antlr.v4.runtime.tree.TerminalNode;

Expand Down Expand Up @@ -65,8 +67,9 @@ public class JPQLNextExpressionVisitorImpl extends JPQLNextParserBaseVisitor<Exp
private final boolean allowOuter;
private final boolean allowQuantifiedPredicates;
private final boolean allowObjectExpression;
private final CharStream input;

public JPQLNextExpressionVisitorImpl(Map<String, Boolean> functions, Map<String, Class<Enum<?>>> enums, Map<String, Class<?>> entities, int minEnumSegmentCount, int minEntitySegmentCount, Map<String, MacroFunction> macros, Set<String> usedMacros, boolean allowOuter, boolean allowQuantifiedPredicates, boolean allowObjectExpression) {
public JPQLNextExpressionVisitorImpl(Map<String, Boolean> functions, Map<String, Class<Enum<?>>> enums, Map<String, Class<?>> entities, int minEnumSegmentCount, int minEntitySegmentCount, Map<String, MacroFunction> macros, Set<String> usedMacros, boolean allowOuter, boolean allowQuantifiedPredicates, boolean allowObjectExpression, CharStream input) {
this.functions = functions;
this.enums = enums;
this.entities = entities;
Expand All @@ -77,6 +80,7 @@ public JPQLNextExpressionVisitorImpl(Map<String, Boolean> functions, Map<String,
this.allowOuter = allowOuter;
this.allowQuantifiedPredicates = allowQuantifiedPredicates;
this.allowObjectExpression = allowObjectExpression;
this.input = input;
}

@Override
Expand Down Expand Up @@ -352,7 +356,7 @@ private Expression handleFunction(String name, boolean distinct, List<Expression
return new FunctionExpression(name, Collections.<Expression>emptyList());
case "outer":
if (!allowOuter) {
throw new SyntaxErrorException("Invalid disallowed use of OUTER in: " + ctx.getText());
throw new SyntaxErrorException("Invalid disallowed use of OUTER in: " + getInputText(ctx));
}
case "concat":
case "substring":
Expand Down Expand Up @@ -443,13 +447,13 @@ private Expression handleMacro(String name, List<Expression> arguments, ParserRu
try {
return macro.apply(arguments);
} catch (RuntimeException ex) {
throw new IllegalArgumentException("Could not apply the macro for the expression: " + ctx.getText(), ex);
throw new IllegalArgumentException("Could not apply the macro for the expression: " + getInputText(ctx), ex);
}
}

private void failDistinct(boolean distinct, ParserRuleContext ctx) {
if (distinct) {
throw new SyntaxErrorException("Invalid use of DISTINCT for function: " + ctx.getText());
throw new SyntaxErrorException("Invalid use of DISTINCT for function: " + getInputText(ctx));
}
}

Expand Down Expand Up @@ -575,7 +579,7 @@ private OrderByItem createOrderByItem(JPQLNextParser.OrderByItemContext ctx) {
boolean asc = true;
boolean nullsFirst = true;
if (ctx.STRING_LITERAL() != null) {
throw new SyntaxErrorException("Collations are not yet supported: " + ctx.getText());
throw new SyntaxErrorException("Collations are not yet supported: " + getInputText(ctx));
}
if (ctx.DESC() != null) {
asc = false;
Expand All @@ -593,10 +597,10 @@ public Expression visitPathExpression(JPQLNextParser.PathExpressionContext ctx)
if (expression instanceof PathExpression) {
List<PathElementExpression> expressions = ((PathExpression) expression).getExpressions();
if (expressions.size() == 1 && expressions.get(0) instanceof TreatExpression) {
throw new SyntaxErrorException("A top level treat expression is not allowed. Consider to further dereference the expression: " + ctx.getText());
throw new SyntaxErrorException("A top level treat expression is not allowed. Consider to further dereference the expression: " + getInputText(ctx));
}
} else if (expression instanceof TreatExpression) {
throw new SyntaxErrorException("A top level treat expression is not allowed. Consider to further dereference the expression: " + ctx.getText());
throw new SyntaxErrorException("A top level treat expression is not allowed. Consider to further dereference the expression: " + getInputText(ctx));
}
}
return expression;
Expand Down Expand Up @@ -870,7 +874,7 @@ public Expression visitQuantifiedSimpleGreaterThanOrEqualPredicate(JPQLNextParse

private void failQuantified(ParserRuleContext ctx, Token qualifier) {
if (qualifier != null && !allowQuantifiedPredicates) {
throw new SyntaxErrorException("The use of quantifiers is not allowed in the context of the expression: " + ctx.getText());
throw new SyntaxErrorException("The use of quantifiers is not allowed in the context of the expression: " + getInputText(ctx));
}
}

Expand Down Expand Up @@ -948,7 +952,7 @@ public Expression visitLikePredicate(JPQLNextParser.LikePredicateContext ctx) {
if (expression instanceof LiteralExpression<?>) {
escapeCharacter = ((LiteralExpression) expression).getValue().toString().charAt(0);
} else {
throw new SyntaxErrorException("Only a character literal is allowed as escape character in like predicate: " + ctx.getText());
throw new SyntaxErrorException("Only a character literal is allowed as escape character in like predicate: " + getInputText(ctx));
}
}
return new LikePredicate(
Expand Down Expand Up @@ -1026,4 +1030,11 @@ private Expression createEntityTypeLiteral(String entityLiteralStr) {
}
return new EntityLiteral(entityType, entityLiteralStr);
}

private String getInputText(ParserRuleContext ctx) {
int from = ctx.start.getStartIndex();
int to = ctx.stop.getStopIndex();
Interval interval = new Interval(from, to);
return input.getText(interval);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public static String prefix(ExpressionFactory ef, String mapping, String prefix)
return prefix + "." + mapping;
} else {
// Since we have functions in here, we have to parse an properly prefix the mapping
Expression expression = ef.createSimpleExpression(mapping, false);
Expression expression = ef.createSimpleExpression(mapping, false, false, true);
SimpleQueryGenerator generator = new PrefixingQueryGenerator(Collections.singletonList(prefix));
StringBuilder sb = new StringBuilder(mapping.length() + prefix.length() + 1);
generator.setQueryBuffer(sb);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ public Map<String, Boolean> getCollectionJoinMappings(ManagedType<?> managedType
return Collections.emptyMap();
}

context.getTypeValidationExpressionFactory().createSimpleExpression(expression, false).accept(visitor);
context.getTypeValidationExpressionFactory().createSimpleExpression(expression, false, false, true).accept(visitor);
Map<String, Boolean> mappings = new HashMap<>();
boolean aggregate = getAttributeType() == AttributeType.SINGULAR;

Expand Down Expand Up @@ -589,7 +589,7 @@ private static void validateTypesCompatible(ManagedType<?> managedType, String e
ScalarTargetResolvingExpressionVisitor visitor = new ScalarTargetResolvingExpressionVisitor(managedType, context.getEntityMetamodel(), context.getJpqlFunctions());

try {
context.getTypeValidationExpressionFactory().createSimpleExpression(expression, false).accept(visitor);
context.getTypeValidationExpressionFactory().createSimpleExpression(expression, false, false, true).accept(visitor);
} catch (SyntaxErrorException ex) {
context.addError("Syntax error in " + expressionLocation + " '" + expression + "' of the " + location + ": " + ex.getMessage());
} catch (IllegalArgumentException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1080,7 +1080,7 @@ private Class<?> getCorrelationBasisType(String correlationBasis, ExpressionFact
}
EntityMetamodel entityMetamodel = evm.getMetamodel().getEntityMetamodel();
ScalarTargetResolvingExpressionVisitor visitor = new ScalarTargetResolvingExpressionVisitor(managedTypeClass, entityMetamodel, evm.getCriteriaBuilderFactory().getRegisteredFunctions());
ef.createSimpleExpression(correlationBasis, false).accept(visitor);
ef.createSimpleExpression(correlationBasis, false, false, true).accept(visitor);
Collection<ScalarTargetResolvingExpressionVisitor.TargetType> possibleTypes = visitor.getPossibleTargetTypes();
if (possibleTypes.size() > 1) {
throw new IllegalArgumentException("The correlation basis '" + correlationBasis + "' is ambiguous in the context of the managed type '" + managedTypeClass.getName() + "'!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public AbstractCorrelationJoinTupleElementMapper(ExpressionFactory ef, String jo
EmbeddingViewJpqlMacro embeddingViewJpqlMacro = (EmbeddingViewJpqlMacro) ef.getDefaultMacroConfiguration().get("EMBEDDING_VIEW").getState()[0];
String oldEmbeddingViewPath = embeddingViewJpqlMacro.getEmbeddingViewPath();
embeddingViewJpqlMacro.setEmbeddingViewPath(embeddingViewPath);
Expression expr = ef.createSimpleExpression(correlationResult, false);
Expression expr = ef.createSimpleExpression(correlationResult, false, false, true);
embeddingViewJpqlMacro.setEmbeddingViewPath(oldEmbeddingViewPath);
SimpleQueryGenerator generator = new PrefixingQueryGenerator(Collections.singletonList(correlationAlias), joinBase, null, null);
generator.setQueryBuffer(sb);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ private void applyMapping(StringBuilder sb, String prefixParts, String mapping,
return;
}
if (prefixParts != null && !prefixParts.isEmpty()) {
Expression expr = ef.createSimpleExpression(mapping, false);
Expression expr = ef.createSimpleExpression(mapping, false, false, true);
EmbeddingViewJpqlMacro embeddingViewJpqlMacro = (EmbeddingViewJpqlMacro) ef.getDefaultMacroConfiguration().get("EMBEDDING_VIEW").getState()[0];
SimpleQueryGenerator generator = new PrefixingQueryGenerator(Collections.singletonList(prefixParts), embeddingViewJpqlMacro.getEmbeddingViewPath(), CorrelatedSubqueryEmbeddingViewJpqlMacro.CORRELATION_EMBEDDING_VIEW_ALIAS, skippedAlias);
generator.setQueryBuffer(sb);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,7 @@ public interface SingleTableBaseView {
public Integer getSub1ValueOrNull();

public SingleTableSimpleBaseView getParent();

@Mapping("TREAT(parent AS SingleTableSub1)")
public SingleTableSimpleSub1View getParentAsSingleTableSub1();
}