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

FEAT: parse tree evaulator and listener #1169

Merged
Show file tree
Hide file tree
Changes from 20 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 @@ -18,6 +18,11 @@ class AndOperator implements Operator<Boolean> {
private static final String DISPLAY_NAME = DataPrepperExpressionParser.VOCABULARY
.getDisplayName(DataPrepperExpressionParser.AND);

@Override
public int getNumberOfOperands() {
return 2;
}

@Override
public boolean shouldEvaluate(final RuleContext ctx) {
return ctx.getRuleIndex() == DataPrepperExpressionParser.RULE_conditionalExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* @since 1.3
* Exception thrown by {@link ParseTreeCoercionService} methods to indicate type coercion failure.
*/
public class ExpressionCoercionException extends Exception {
public class ExpressionCoercionException extends RuntimeException {
public ExpressionCoercionException(final String message) {
super(message);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ public GenericEqualOperator(final int symbol, BiPredicate<Object, Object> operat
this.operation = operation;
}

@Override
public int getNumberOfOperands() {
return 2;
}

@Override
public boolean shouldEvaluate(final RuleContext ctx) {
return ctx.getRuleIndex() == DataPrepperExpressionParser.RULE_equalityOperatorExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ public GenericInSetOperator(final int symbol, BiPredicate<Object, Object> operat
this.operation = operation;
}

@Override
public int getNumberOfOperands() {
return 2;
}

@Override
public boolean shouldEvaluate(final RuleContext ctx) {
return ctx.getRuleIndex() == DataPrepperExpressionParser.RULE_setOperatorExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ public GenericRegexMatchOperator(final int symbol, BiPredicate<Object, Object> o
this.operation = operation;
}

@Override
public int getNumberOfOperands() {
return 2;
}

@Override
public boolean shouldEvaluate(final RuleContext ctx) {
return ctx.getRuleIndex() == DataPrepperExpressionParser.RULE_regexOperatorExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ class NotOperator implements Operator<Boolean> {
private static final String DISPLAY_NAME = DataPrepperExpressionParser.VOCABULARY
.getDisplayName(DataPrepperExpressionParser.NOT);

@Override
public int getNumberOfOperands() {
return 1;
}

@Override
public boolean shouldEvaluate(final RuleContext ctx) {
return ctx.getRuleIndex() == DataPrepperExpressionParser.RULE_unaryNotOperatorExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ public NumericCompareOperator(final int symbol,
}


@Override
public int getNumberOfOperands() {
return 2;
}

@Override
public boolean shouldEvaluate(final RuleContext ctx) {
return ctx.getRuleIndex() == DataPrepperExpressionParser.RULE_relationalOperatorExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import org.antlr.v4.runtime.RuleContext;

interface Operator<T> {
int getNumberOfOperands();

boolean shouldEvaluate(final RuleContext ctx);

int getSymbol();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,8 @@ public Operator<?> getOperator(final int symbol) {
}
return operator;
}

public boolean containsOperator(final int symbol) {
return symbolToOperators.containsKey(symbol);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ class OrOperator implements Operator<Boolean> {
private static final String DISPLAY_NAME = DataPrepperExpressionParser.VOCABULARY
.getDisplayName(DataPrepperExpressionParser.OR);

@Override
public int getNumberOfOperands() {
return 2;
}

@Override
public boolean shouldEvaluate(final RuleContext ctx) {
return ctx.getRuleIndex() == DataPrepperExpressionParser.RULE_conditionalExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

@Named
class ParseTreeCoercionService {
public Object coercePrimaryTerminalNode(final TerminalNode node, final Event event) throws ExpressionCoercionException {
public Object coercePrimaryTerminalNode(final TerminalNode node, final Event event) {
final int nodeType = node.getSymbol().getType();
final String nodeStringValue = node.getText();
switch (nodeType) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.dataprepper.expression;

import com.amazon.dataprepper.model.event.Event;
import org.antlr.v4.runtime.tree.ParseTree;
import org.antlr.v4.runtime.tree.ParseTreeWalker;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.inject.Inject;
import javax.inject.Named;

@Named
class ParseTreeEvaluator implements Evaluator<ParseTree, Event> {
private static final Logger LOG = LoggerFactory.getLogger(ParseTreeEvaluator.class);

private final OperatorProvider operatorProvider;
private final ParseTreeWalker walker;
private final ParseTreeCoercionService coercionService;

@Inject
public ParseTreeEvaluator(final OperatorProvider operatorProvider, final ParseTreeWalker walker,
final ParseTreeCoercionService coercionService) {
this.operatorProvider = operatorProvider;
this.walker = walker;
this.coercionService = coercionService;
}

@Override
public Boolean evaluate(ParseTree parseTree, Event event) {
try {
final ParseTreeEvaluatorListener listener = new ParseTreeEvaluatorListener(operatorProvider, coercionService, event);
walker.walk(listener, parseTree);
return coercionService.coerce(listener.getResult(), Boolean.class);
} catch (final Exception e) {
LOG.error("Unable to evaluate event", e);
throw new ExpressionEvaluationException(e.getMessage(), e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.dataprepper.expression;

import com.amazon.dataprepper.model.event.Event;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.tree.ErrorNode;
import org.antlr.v4.runtime.tree.TerminalNode;
import org.opensearch.dataprepper.expression.antlr.DataPrepperExpressionBaseListener;
import org.opensearch.dataprepper.expression.antlr.DataPrepperExpressionListener;
import org.opensearch.dataprepper.expression.antlr.DataPrepperExpressionParser;

import java.util.Stack;

/**
* @since 1.3
* This listener implements {@link DataPrepperExpressionListener} to provide callbacks to handle evaluation of
* {@link org.antlr.v4.runtime.tree.ParseTree} representation of an expression while {@link org.antlr.v4.runtime.tree.ParseTreeWalker}
* traverses through the {@link org.antlr.v4.runtime.tree.ParseTree}.
*
* Use case:
* ParseTreeWalker walker = new ParseTreeWalker();
* ParseTreeEvaluatorListener listener = new ParseTreeEvaluatorListener(...);
* walker.walk(listener, ...);
* final Object result = listener.getResult();
*/
class ParseTreeEvaluatorListener extends DataPrepperExpressionBaseListener {
Copy link
Member

Choose a reason for hiding this comment

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

Nice! I like how this came out.

Copy link
Member

Choose a reason for hiding this comment

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

Nice! I like how this came out.


private final OperatorProvider operatorProvider;
private final ParseTreeCoercionService coercionService;
private final Stack<Integer> operatorSymbolStack;
private final Stack<Object> operandStack;
private final Event event;

public ParseTreeEvaluatorListener(final OperatorProvider operatorProvider,
final ParseTreeCoercionService coercionService,
final Event event) {
this.coercionService = coercionService;
this.operatorProvider = operatorProvider;
this.event = event;
operatorSymbolStack = new Stack<>();
operandStack = new Stack<>();
}

public Object getResult() {
if (operandStack.size() != 1) {
throw new IllegalStateException("The ParseTreeEvaluatorListener has not been walked through exactly once by " +
"a ParseTreeWalker.");
}
return operandStack.peek();
Copy link
Member

Choose a reason for hiding this comment

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

It might be safer to pop to prevent reuse of a listener considering each evaluate call needs a listener.

Copy link
Collaborator Author

@chenqi0805 chenqi0805 Mar 10, 2022

Choose a reason for hiding this comment

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

The reason I did not use pop is based off idempotency. It might be a little weird that the second getResult() call throws exception when called in succession.

}

@Override
public void visitTerminal(TerminalNode node) {
final int nodeType = node.getSymbol().getType();
if (nodeType == DataPrepperExpressionParser.EOF) {
return;
}
if (operatorProvider.containsOperator(nodeType) || nodeType == DataPrepperExpressionParser.LPAREN) {
operatorSymbolStack.push(nodeType);
} else if (nodeType == DataPrepperExpressionParser.RPAREN) {
// pop LPAREN at operatorSymbolStack top
operatorSymbolStack.pop();
Copy link
Member

Choose a reason for hiding this comment

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

Consider verifying the top of the stack is a LPAREN.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean throw exception if it is not LPAREN? AFAICT, it should only happen if parentheses in the expression are unbalanced, which should already be ruled out by parser?

} else {
final Object arg = coercionService.coercePrimaryTerminalNode(node, event);
operandStack.push(arg);
}
}

@Override
public void visitErrorNode(ErrorNode node) {
throw new RuntimeException("Hit error node in the parse tree: " + node.getText());
}

@Override
public void exitEveryRule(ParserRuleContext ctx) {
if (!operatorSymbolStack.isEmpty()) {
final int operatorSymbol = operatorSymbolStack.peek();
if (operatorSymbol != DataPrepperExpressionParser.LPAREN) {
final Operator<?> op = operatorProvider.getOperator(operatorSymbol);
if (op.shouldEvaluate(ctx)) {
operatorSymbolStack.pop();
try {
performSingleOperation(op);
} catch (final Exception e) {
throw new ExpressionEvaluationException("Unable to evaluate the part of input statement: "
+ getPartialStatementFromContext(ctx), e);
}
}
}
}
}

private void performSingleOperation(final Operator<?> operator) {
final int numOfArgs = operator.getNumberOfOperands();
final Object[] args = new Object[numOfArgs];
for (int i = numOfArgs - 1; i >= 0; i--) {
Copy link
Member

Choose a reason for hiding this comment

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

Checking operandStack.size() >= numOfArgs and throwing a more informative error message may lead to a better customer experience.

Copy link
Collaborator Author

@chenqi0805 chenqi0805 Mar 10, 2022

Choose a reason for hiding this comment

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

Not sure if it is necessary. Here is the reasoning: I suppose if the input parseTree is valid, we should never hit this error unless the evaluation algorithm is incorrect and fails to apply to certain corner cases. I do not think we should use exception or error message to guard against incorrect implementation. Also, the error message thrown here is not that helpful to cx on troubleshooting their if condition statement.

args[i] = operandStack.pop();
}
final Object result = operator.evaluate(args);
operandStack.push(result);
}

private String getPartialStatementFromContext(final ParserRuleContext ctx) {
final Token startToken = ctx.getStart();
final Token stopToken = ctx.getStop();
final String fullStatement = startToken.getInputStream().toString();
return fullStatement.substring(startToken.getStartIndex(), stopToken.getStopIndex() + 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ class AndOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(2));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_conditionalExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ class EqualOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(2));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_equalityOperatorExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ class GreaterThanOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(2));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_relationalOperatorExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ class GreaterThanOrEqualOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(2));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_relationalOperatorExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ class InSetOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(2));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_setOperatorExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ class LessThanOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(2));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_relationalOperatorExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ class LessThanOrEqualOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(2));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_relationalOperatorExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ class NotEqualOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(2));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_equalityOperatorExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ class NotInSetOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(2));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_setOperatorExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ class NotOperatorTest {
@Mock
private ParserRuleContext ctx;

@Test
void testGetNumberOfOperands() {
assertThat(objectUnderTest.getNumberOfOperands(), is(1));
}

@Test
void testShouldEvaluate() {
when(ctx.getRuleIndex()).thenReturn(DataPrepperExpressionParser.RULE_unaryNotOperatorExpression);
Expand Down
Loading