diff --git a/ksql-engine/src/test/resources/query-validation-tests/project-filter.json b/ksql-engine/src/test/resources/query-validation-tests/project-filter.json index 777d2b9dc65b..9268e325a1ef 100644 --- a/ksql-engine/src/test/resources/query-validation-tests/project-filter.json +++ b/ksql-engine/src/test/resources/query-validation-tests/project-filter.json @@ -60,6 +60,29 @@ {"topic": "S1", "key": 0, "timestamp": 0, "value": {"ID":4}} ] }, + { + "name": "WHERE with many comparisons. This tests the fix for #1784", + "statements": [ + "CREATE STREAM events (id int, field0 varchar) WITH (KAFKA_TOPIC='events', VALUE_FORMAT='json');", + "CREATE STREAM eventstest AS SELECT id, 'x_0' AS field1, field0 FROM events WHERE ((id=1 OR id=2 OR id=3 OR id=4) AND (field0='0x10' OR field0='0x11' OR field0='0x12' OR field0='0x13' OR field0='0x14' OR field0='0x15' OR field0='0x16' OR field0='0x17' OR field0='0x18' OR field0='0x19' OR field0='0x1A' OR field0='0x1B' OR field0='0x1C' OR field0='0x1D' OR field0='0x1E' OR field0='0x1F' OR field0='0x20' OR field0='0x21' OR field0='0x22' OR field0='0x23' OR field0='0x24' OR field0='0x25' OR field0='0x26' OR field0='0x27' OR field0='0x28' OR field0='0x29' OR field0='0x2A' OR field0='0x2B' OR field0='0x2C' OR field0='0x2D' OR field0='0x2E' OR field0='0x2F' OR field0='0x30' OR field0='0x31' OR field0='0x32' OR field0='0x33' OR field0='0x34' OR field0='0x35' OR field0='0x36' OR field0='0x37' OR field0='0x38' OR field0='0x39' OR field0='0x3A' OR field0='0x3B' OR field0='0x3C' OR field0='0x3D' OR field0='0x3E' OR field0='0x3F'));" + ], + "inputs": [ + { + "topic": "events", + "key": 0, + "value": {"id": 1, "field0": "0x10"}, + "timestamp": 0 + } + ], + "outputs": [ + { + "topic": "EVENTSTEST", + "key": 0, + "value": {"ID": 1, "FIELD1": "x_0", "FIELD0": "0x10"}, + "timestamp": 0 + } + ] + }, { "name": "project and negative filter", "statements": [ diff --git a/ksql-parser/src/main/java/io/confluent/ksql/parser/rewrite/StatementRewriter.java b/ksql-parser/src/main/java/io/confluent/ksql/parser/rewrite/StatementRewriter.java index 1d41185b2cb5..f52394344979 100644 --- a/ksql-parser/src/main/java/io/confluent/ksql/parser/rewrite/StatementRewriter.java +++ b/ksql-parser/src/main/java/io/confluent/ksql/parser/rewrite/StatementRewriter.java @@ -93,7 +93,6 @@ import io.confluent.ksql.parser.tree.WindowFrame; import io.confluent.ksql.parser.tree.WithQuery; import io.confluent.ksql.util.KsqlException; - import java.util.Map; import java.util.Map.Entry; import java.util.Optional; @@ -105,7 +104,10 @@ * the imput one. If you want to rewrite a query by changing the AST you can inherit from this * class and implemet the changes for the nodes you need. The newly generated tree will include * your changes and the rest of the tree will remain the same. - * + * + *

Implementation note: make sure if you change this class you create each node only once in the + * if/else blocks. + *

*/ // CHECKSTYLE_RULES.OFF: ClassDataAbstractionCoupling public class StatementRewriter extends DefaultAstVisitor { @@ -132,67 +134,68 @@ protected Node visitArithmeticBinary( final ArithmeticBinaryExpression node, final Object context ) { - return node.getLocation() - .map(location -> - new ArithmeticBinaryExpression(location, node.getType(), - (Expression) process(node.getLeft(), context), - (Expression) process(node.getRight(), context))) - .orElse(new ArithmeticBinaryExpression(node.getType(), - (Expression) process(node.getLeft(), context), - (Expression) process(node.getRight(), context))); - + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new ArithmeticBinaryExpression(node.getLocation().get(), + node.getType(), + (Expression) process(node.getLeft(), context), + (Expression) process(node.getRight(), context)); + } else { + return new ArithmeticBinaryExpression(node.getType(), + (Expression) process(node.getLeft(), context), + (Expression) process(node.getRight(), context)); + } } protected Node visitBetweenPredicate(final BetweenPredicate node, final Object context) { - return node.getLocation() - .map(location -> - new BetweenPredicate(node.getLocation().get(), - (Expression) process(node.getValue(), context), - (Expression) process(node.getMin(), context), - (Expression) process(node.getMax(), context)) - ) - .orElse( - new BetweenPredicate((Expression) process(node.getValue(), context), - (Expression) process(node.getMin(), context), - (Expression) process(node.getMax(), context)) - ); - + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new BetweenPredicate(node.getLocation().get(), + (Expression) process(node.getValue(), context), + (Expression) process(node.getMin(), context), + (Expression) process(node.getMax(), context)); + } else { + return new BetweenPredicate((Expression) process(node.getValue(), context), + (Expression) process(node.getMin(), context), + (Expression) process(node.getMax(), context)); + } } protected Node visitComparisonExpression(final ComparisonExpression node, final Object context) { - return node.getLocation() - .map(location -> - new ComparisonExpression(node.getLocation().get(), - node.getType(), - (Expression) process(node.getLeft(), context), - (Expression) process(node.getRight(), context)) - ) - .orElse( - new ComparisonExpression(node.getType(), - (Expression) process(node.getLeft(), context), - (Expression) process(node.getRight(), context)) - ); - + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new ComparisonExpression(node.getLocation().get(), + node.getType(), + (Expression) process(node.getLeft(), context), + (Expression) process(node.getRight(), context)); + } else { + return new ComparisonExpression(node.getType(), + (Expression) process(node.getLeft(), context), + (Expression) process(node.getRight(), context)); + } } protected Node visitDoubleLiteral(final DoubleLiteral node, final Object context) { - return node.getLocation() - .map(location -> - new DoubleLiteral(node.getLocation().get(), String.valueOf(node.getValue())) - ) - .orElse( - new DoubleLiteral(String.valueOf(node.getValue())) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new DoubleLiteral(node.getLocation().get(), String.valueOf(node.getValue())); + } else { + return new DoubleLiteral(String.valueOf(node.getValue())); + } } protected Node visitDecimalLiteral(final DecimalLiteral node, final Object context) { - return node.getLocation() - .map(location -> - new DecimalLiteral(node.getLocation().get(), node.getValue()) - ) - .orElse( - new DecimalLiteral(node.getValue()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new DecimalLiteral(node.getLocation().get(), node.getValue()); + } else { + return new DecimalLiteral(node.getValue()); + } } protected Node visitStatements(final Statements node, final Object context) { @@ -211,50 +214,48 @@ protected Node visitQuery(final Query node, final Object context) { } protected Node visitTimeLiteral(final TimeLiteral node, final Object context) { - return node.getLocation() - .map(location -> - new TimeLiteral(node.getLocation().get(), node.getValue()) - ) - .orElse( - new TimeLiteral(node.getValue()) - ); - + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new TimeLiteral(node.getLocation().get(), node.getValue()); + } else { + return new TimeLiteral(node.getValue()); + } } protected Node visitWithQuery(final WithQuery node, final Object context) { - return node.getLocation() - .map(location -> - new WithQuery(node.getLocation().get(), - node.getName(), - (Query) process(node.getQuery(), context), - node.getColumnNames()) - ) - .orElse( - new WithQuery(node.getName(), - (Query) process(node.getQuery(), context), - node.getColumnNames()) - ); - + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new WithQuery(node.getLocation().get(), + node.getName(), + (Query) process(node.getQuery(), context), + node.getColumnNames()); + } else { + return new WithQuery(node.getName(), + (Query) process(node.getQuery(), context), + node.getColumnNames()); + } } protected Node visitSelect(final Select node, final Object context) { - return node.getLocation() - .map(location -> - new Select(node.getLocation().get(), - node.isDistinct(), - node.getSelectItems() - .stream() - .map(selectItem -> (SelectItem) process(selectItem, context)) - .collect(Collectors.toList())) - ) - .orElse( - new Select(node.isDistinct(), - node.getSelectItems() - .stream() - .map(selectItem -> (SelectItem) process(selectItem, context)) - .collect(Collectors.toList()) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new Select(node.getLocation().get(), + node.isDistinct(), + node.getSelectItems() + .stream() + .map(selectItem -> (SelectItem) process(selectItem, context)) + .collect(Collectors.toList())); + } else { + return new Select(node.isDistinct(), + node.getSelectItems() + .stream() + .map(selectItem -> (SelectItem) process(selectItem, context)) + .collect(Collectors.toList()) + ); + } } protected Node visitQuerySpecification(final QuerySpecification node, final Object context) { @@ -274,89 +275,89 @@ protected Node visitQuerySpecification(final QuerySpecification node, final Obje ? Optional.ofNullable((Expression) process(node.getHaving().get(), context)) : Optional.empty(); - return node.getLocation() - .map(location -> - new QuerySpecification( - node.getLocation().get(), - (Select) process(node.getSelect(), context), - (Relation) process(node.getInto(), context), - node.isShouldCreateInto(), - (Relation) process(node.getFrom(), context), - windowExpression, - where, - groupBy, - having, - node.getLimit() - ) - ) - .orElse( - new QuerySpecification( - (Select) process(node.getSelect(), context), - (Relation) process(node.getInto(), context), - node.isShouldCreateInto(), - (Relation) process(node.getFrom(), context), - windowExpression, - where, - groupBy, - having, - node.getLimit() - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new QuerySpecification( + node.getLocation().get(), + (Select) process(node.getSelect(), context), + (Relation) process(node.getInto(), context), + node.isShouldCreateInto(), + (Relation) process(node.getFrom(), context), + windowExpression, + where, + groupBy, + having, + node.getLimit() + ); + } else { + return new QuerySpecification( + (Select) process(node.getSelect(), context), + (Relation) process(node.getInto(), context), + node.isShouldCreateInto(), + (Relation) process(node.getFrom(), context), + windowExpression, + where, + groupBy, + having, + node.getLimit() + ); + } } protected Node visitTimestampLiteral(final TimestampLiteral node, final Object context) { - return node.getLocation() - .map(location -> - new TimeLiteral(node.getLocation().get(), node.getValue()) - ) - .orElse( - new TimeLiteral(node.getValue()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new TimeLiteral(node.getLocation().get(), node.getValue()); + } else { + return new TimeLiteral(node.getValue()); + } } protected Node visitWhenClause(final WhenClause node, final Object context) { - return node.getLocation() - .map(location -> - new WhenClause(node.getLocation().get(), - (Expression) process(node.getOperand(), context), - (Expression) process(node.getResult(), context)) - ) - .orElse( - new WhenClause((Expression) process(node.getOperand(), context), - (Expression) process(node.getResult(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new WhenClause(node.getLocation().get(), + (Expression) process(node.getOperand(), context), + (Expression) process(node.getResult(), context)); + } else { + return new WhenClause((Expression) process(node.getOperand(), context), + (Expression) process(node.getResult(), context)); + } } protected Node visitIntervalLiteral(final IntervalLiteral node, final Object context) { - return node.getLocation() - .map(location -> - new IntervalLiteral(node.getLocation().get(), - node.getValue(), - node.getSign(), - node.getStartField(), - node.getEndField() - ) - ) - .orElse( - new IntervalLiteral(node.getValue(), - node.getSign(), - node.getStartField(), - node.getEndField() - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new IntervalLiteral(node.getLocation().get(), + node.getValue(), + node.getSign(), + node.getStartField(), + node.getEndField() + ); + } else { + return new IntervalLiteral(node.getValue(), + node.getSign(), + node.getStartField(), + node.getEndField() + ); + } } protected Node visitInPredicate(final InPredicate node, final Object context) { - return node.getLocation() - .map(location -> - new InPredicate(node.getLocation().get(), - (Expression) process(node.getValue(), context), - (Expression) process(node.getValueList(), context)) - ) - .orElse( - new InPredicate((Expression) process(node.getValue(), context), - (Expression) process(node.getValueList(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new InPredicate(node.getLocation().get(), + (Expression) process(node.getValue(), context), + (Expression) process(node.getValueList(), context)); + } else { + return new InPredicate((Expression) process(node.getValue(), context), + (Expression) process(node.getValueList(), context)); + } } protected Node visitFunctionCall(final FunctionCall node, final Object context) { @@ -364,209 +365,210 @@ protected Node visitFunctionCall(final FunctionCall node, final Object context) ? Optional.ofNullable((Window) process(node.getWindow().get(), context)) : Optional.empty(); - return node.getLocation() - .map(location -> - new FunctionCall(node.getLocation().get(), - node.getName(), - window, - node.isDistinct(), - node.getArguments() - .stream() - .map(arg -> (Expression) process(arg, context)) - .collect(Collectors.toList()) - ) - ) - .orElse( - new FunctionCall(node.getName(), - window, - node.isDistinct(), - node.getArguments() - .stream() - .map(arg -> (Expression) process(arg, context)) - .collect(Collectors.toList()) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new FunctionCall(node.getLocation().get(), + node.getName(), + window, + node.isDistinct(), + node.getArguments() + .stream() + .map(arg -> (Expression) process(arg, context)) + .collect(Collectors.toList()) + ); + } else { + return new FunctionCall(node.getName(), + window, + node.isDistinct(), + node.getArguments() + .stream() + .map(arg -> (Expression) process(arg, context)) + .collect(Collectors.toList()) + ); + } } protected Node visitSimpleCaseExpression(final SimpleCaseExpression node, final Object context) { final Optional defaultValue = node.getDefaultValue().isPresent() ? Optional.ofNullable((Expression) process(node.getDefaultValue().get(), context)) : Optional.empty(); - return node.getLocation() - .map(location -> - new SimpleCaseExpression(node.getLocation().get(), - (Expression) process(node.getOperand(), context), - node.getWhenClauses() - .stream() - .map(whenClause -> - (WhenClause) process(whenClause, context)) - .collect(Collectors.toList()), - defaultValue - ) - ) - .orElse( - new SimpleCaseExpression((Expression) process(node.getOperand(), context), - node.getWhenClauses() - .stream() - .map(whenClause -> - (WhenClause) process(whenClause, context)) - .collect(Collectors.toList()), - defaultValue - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new SimpleCaseExpression(node.getLocation().get(), + (Expression) process(node.getOperand(), context), + node.getWhenClauses() + .stream() + .map(whenClause -> + (WhenClause) process(whenClause, context)) + .collect(Collectors.toList()), + defaultValue + ); + } else { + return new SimpleCaseExpression((Expression) process(node.getOperand(), context), + node.getWhenClauses() + .stream() + .map(whenClause -> + (WhenClause) process(whenClause, context)) + .collect(Collectors.toList()), + defaultValue + ); + } } protected Node visitStringLiteral(final StringLiteral node, final Object context) { - return node.getLocation() - .map(location -> - new StringLiteral(node.getLocation().get(), - node.getValue()) - ) - .orElse( - new StringLiteral(node.getValue()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new StringLiteral(node.getLocation().get(), + node.getValue()); + } else { + return new StringLiteral(node.getValue()); + } } protected Node visitBinaryLiteral(final BinaryLiteral node, final Object context) { // May need some changes. - return node.getLocation() - .map(location -> - new BinaryLiteral(node.getLocation().get(), node.getValue().toString()) - ) - .orElse( - new BinaryLiteral(node.getValue().toString()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new BinaryLiteral(node.getLocation().get(), node.getValue().toString()); + } else { + return new BinaryLiteral(node.getValue().toString()); + } } protected Node visitBooleanLiteral(final BooleanLiteral node, final Object context) { - return node.getLocation() - .map(location -> - new BooleanLiteral(node.getLocation().get(), String.valueOf(node.getValue())) - ) - .orElse( - new BooleanLiteral(String.valueOf(node.getValue())) - ); - + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new BooleanLiteral(node.getLocation().get(), String.valueOf(node.getValue())); + } else { + return new BooleanLiteral(String.valueOf(node.getValue())); + } } protected Node visitInListExpression(final InListExpression node, final Object context) { - return node.getLocation() - .map(location -> - new InListExpression(node.getLocation().get(), - node.getValues().stream() - .map(value -> (Expression) process(value, context)) - .collect(Collectors.toList()) - ) - ) - .orElse( - new InListExpression(node.getValues().stream() - .map(value -> (Expression) process(value, context)) - .collect(Collectors.toList()) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new InListExpression(node.getLocation().get(), + node.getValues().stream() + .map(value -> (Expression) process(value, context)) + .collect(Collectors.toList()) + ); + } else { + return new InListExpression(node.getValues().stream() + .map(value -> (Expression) process(value, context)) + .collect(Collectors.toList()) + ); + } } protected Node visitQualifiedNameReference( final QualifiedNameReference node, final Object context ) { - return node.getLocation() - .map(location -> - new QualifiedNameReference(node.getLocation().get(), - node.getName()) - ) - .orElse( - new QualifiedNameReference(node.getName()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new QualifiedNameReference(node.getLocation().get(), + node.getName()); + } else { + return new QualifiedNameReference(node.getName()); + } } protected Node visitDereferenceExpression( final DereferenceExpression node, final Object context ) { - return node.getLocation() - .map(location -> - new DereferenceExpression(node.getLocation().get(), - (Expression) process(node.getBase(), context), - node.getFieldName() - ) - ) - .orElse( - new DereferenceExpression((Expression) process(node.getBase(), context), - node.getFieldName() - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new DereferenceExpression(node.getLocation().get(), + (Expression) process(node.getBase(), context), + node.getFieldName() + ); + } else { + return new DereferenceExpression((Expression) process(node.getBase(), context), + node.getFieldName() + ); + } } protected Node visitNullIfExpression(final NullIfExpression node, final Object context) { - return node.getLocation() - .map(location -> - new NullIfExpression(node.getLocation().get(), - (Expression) process(node.getFirst(), context), - (Expression) process(node.getSecond(), context)) - ) - .orElse( - new NullIfExpression((Expression) process(node.getFirst(), context), - (Expression) process(node.getSecond(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new NullIfExpression(node.getLocation().get(), + (Expression) process(node.getFirst(), context), + (Expression) process(node.getSecond(), context)); + } else { + return new NullIfExpression((Expression) process(node.getFirst(), context), + (Expression) process(node.getSecond(), context)); + } } protected Node visitNullLiteral(final NullLiteral node, final Object context) { - return node.getLocation() - .map(location -> - new NullLiteral(node.getLocation().get()) - ) - .orElse( - new NullLiteral() - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new NullLiteral(node.getLocation().get()); + } else { + return new NullLiteral(); + } } protected Node visitArithmeticUnary(final ArithmeticUnaryExpression node, final Object context) { - return node.getLocation() - .map(location -> - new ArithmeticUnaryExpression( - node.getLocation().get(), - node.getSign(), - (Expression) process(node.getValue(), context) - ) - ) - .orElse( - new ArithmeticUnaryExpression( - node.getSign(), - (Expression) process(node.getValue(), context) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new ArithmeticUnaryExpression( + node.getLocation().get(), + node.getSign(), + (Expression) process(node.getValue(), context) + ); + } else { + return new ArithmeticUnaryExpression( + node.getSign(), + (Expression) process(node.getValue(), context) + ); + } } protected Node visitNotExpression(final NotExpression node, final Object context) { - return node.getLocation() - .map(location -> - new NotExpression(node.getLocation().get(), - (Expression) process(node.getValue(), context) - ) - ) - .orElse( - new NotExpression((Expression) process(node.getValue(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new NotExpression(node.getLocation().get(), + (Expression) process(node.getValue(), context) + ); + } else { + return new NotExpression((Expression) process(node.getValue(), context)); + } } protected Node visitSingleColumn(final SingleColumn node, final Object context) { - return node.getLocation() - .map(location -> - new SingleColumn(node.getLocation().get(), - (Expression) process(node.getExpression(), context), - node.getAlias() - ) - ) - .orElse( - new SingleColumn( - (Expression) process(node.getExpression(), context), - node.getAlias() - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new SingleColumn(node.getLocation().get(), + (Expression) process(node.getExpression(), context), + node.getAlias() + ); + } else { + return new SingleColumn( + (Expression) process(node.getExpression(), context), + node.getAlias() + ); + } } protected Node visitAllColumns(final AllColumns node, final Object context) { + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) if (node.getLocation().isPresent()) { if (node.getPrefix().isPresent()) { return new AllColumns(node.getLocation().get(), node.getPrefix().get()); @@ -592,125 +594,125 @@ protected Node visitSearchedCaseExpression( final Optional defaultValue = node.getDefaultValue().isPresent() ? Optional.ofNullable((Expression) process(node.getDefaultValue().get(), context)) : Optional.empty(); - return node.getLocation() - .map(location -> - new SearchedCaseExpression( - node.getLocation().get(), - node.getWhenClauses() - .stream().map(whenClause -> (WhenClause) process(whenClause, context)) - .collect(Collectors.toList()), - defaultValue - ) - ) - .orElse( - new SearchedCaseExpression( - node.getWhenClauses() - .stream().map(whenClause -> (WhenClause) process(whenClause, context)) - .collect(Collectors.toList()), - defaultValue - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new SearchedCaseExpression( + node.getLocation().get(), + node.getWhenClauses() + .stream().map(whenClause -> (WhenClause) process(whenClause, context)) + .collect(Collectors.toList()), + defaultValue + ); + } else { + return new SearchedCaseExpression( + node.getWhenClauses() + .stream().map(whenClause -> (WhenClause) process(whenClause, context)) + .collect(Collectors.toList()), + defaultValue + ); + } } protected Node visitLikePredicate(final LikePredicate node, final Object context) { - return node.getLocation() - .map(location -> - new LikePredicate(node.getLocation().get(), - (Expression) process(node.getValue(), context), - (Expression) process(node.getPattern(), context), - node.getEscape() != null - ? (Expression) process(node.getEscape(), context) - : null - ) - ) - .orElse( - new LikePredicate((Expression) process(node.getValue(), context), - (Expression) process(node.getPattern(), context), - node.getEscape() != null - ? (Expression) process(node.getEscape(), context) - : null - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new LikePredicate(node.getLocation().get(), + (Expression) process(node.getValue(), context), + (Expression) process(node.getPattern(), context), + node.getEscape() != null + ? (Expression) process(node.getEscape(), context) + : null + ); + } else { + return new LikePredicate((Expression) process(node.getValue(), context), + (Expression) process(node.getPattern(), context), + node.getEscape() != null + ? (Expression) process(node.getEscape(), context) + : null + ); + } } protected Node visitIsNotNullPredicate(final IsNotNullPredicate node, final Object context) { - return node.getLocation() - .map(location -> - new IsNotNullPredicate(node.getLocation().get(), - (Expression) process(node.getValue(), context) - ) - ) - .orElse( - new IsNotNullPredicate((Expression) process(node.getValue(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new IsNotNullPredicate(node.getLocation().get(), + (Expression) process(node.getValue(), context) + ); + } else { + return new IsNotNullPredicate((Expression) process(node.getValue(), context)); + } } protected Node visitIsNullPredicate(final IsNullPredicate node, final Object context) { - return node.getLocation() - .map(location -> - new IsNullPredicate(node.getLocation().get(), - (Expression) process(node.getValue(), context) - ) - ) - .orElse( - new IsNullPredicate((Expression) process(node.getValue(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new IsNullPredicate(node.getLocation().get(), + (Expression) process(node.getValue(), context) + ); + } else { + return new IsNullPredicate((Expression) process(node.getValue(), context)); + } } protected Node visitSubscriptExpression(final SubscriptExpression node, final Object context) { - return node.getLocation() - .map(location -> - new SubscriptExpression(node.getLocation().get(), - (Expression) process(node.getBase(), context), - (Expression) process(node.getIndex(), context) - ) - ) - .orElse( - new SubscriptExpression((Expression) process(node.getBase(), context), - (Expression) process(node.getIndex(), context) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new SubscriptExpression(node.getLocation().get(), + (Expression) process(node.getBase(), context), + (Expression) process(node.getIndex(), context) + ); + } else { + return new SubscriptExpression((Expression) process(node.getBase(), context), + (Expression) process(node.getIndex(), context) + ); + } } protected Node visitLongLiteral(final LongLiteral node, final Object context) { - return node.getLocation() - .map(location -> - new LongLiteral(node.getLocation().get(), node.getValue()) - ) - .orElse( - new LongLiteral(node.getValue()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new LongLiteral(node.getLocation().get(), node.getValue()); + } else { + return new LongLiteral(node.getValue()); + } } protected Node visitLogicalBinaryExpression( final LogicalBinaryExpression node, final Object context ) { - return node.getLocation() - .map(location -> - new LogicalBinaryExpression(node.getLocation().get(), - node.getType(), - (Expression) process(node.getLeft(), context), - (Expression) process(node.getRight(), context) - ) - ) - .orElse( - new LogicalBinaryExpression(node.getType(), - (Expression) process(node.getLeft(), context), - (Expression) process(node.getRight(), context) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new LogicalBinaryExpression(node.getLocation().get(), + node.getType(), + (Expression) process(node.getLeft(), context), + (Expression) process(node.getRight(), context) + ); + } else { + return new LogicalBinaryExpression(node.getType(), + (Expression) process(node.getLeft(), context), + (Expression) process(node.getRight(), context) + ); + } } protected Node visitSubqueryExpression(final SubqueryExpression node, final Object context) { - return node.getLocation() - .map(location -> - new SubqueryExpression(node.getLocation().get(), - (Query) process(node.getQuery(), context)) - ) - .orElse( - new SubqueryExpression((Query) process(node.getQuery(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new SubqueryExpression(node.getLocation().get(), + (Query) process(node.getQuery(), context)); + } else { + return new SubqueryExpression((Query) process(node.getQuery(), context)); + } } protected Node visitTable(final Table node, final Object context) { @@ -719,122 +721,122 @@ protected Node visitTable(final Table node, final Object context) { Map.Entry::getKey, e -> (Expression) process(e.getValue(), context) )); - return node.getLocation() - .map(location -> - new Table(node.getLocation().get(), - node.getName(), - node.isStdOut(), - properties) - ) - .orElse( - new Table(node.getName(), - node.isStdOut(), - properties) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new Table(node.getLocation().get(), + node.getName(), + node.isStdOut(), + properties); + } else { + return new Table(node.getName(), + node.isStdOut(), + properties); + } } protected Node visitValues(final Values node, final Object context) { - return node.getLocation() - .map(location -> - new Values(node.getLocation().get(), - node.getRows().stream() - .map(row -> (Expression) process(row, context)) - .collect(Collectors.toList()) - ) - ) - .orElse( - new Values(node.getRows().stream() - .map(row -> (Expression) process(row, context)) - .collect(Collectors.toList()) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new Values(node.getLocation().get(), + node.getRows().stream() + .map(row -> (Expression) process(row, context)) + .collect(Collectors.toList()) + ); + } else { + return new Values(node.getRows().stream() + .map(row -> (Expression) process(row, context)) + .collect(Collectors.toList()) + ); + } } protected Node visitStruct(final Struct node, final Object context) { - return node.getLocation() - .map(location -> - new Struct(node.getLocation().get(), - node.getItems()) - ) - .orElse( - new Struct(node.getItems()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new Struct(node.getLocation().get(), + node.getItems()); + } else { + return new Struct(node.getItems()); + } } protected Node visitTableSubquery(final TableSubquery node, final Object context) { - return node.getLocation() - .map(location -> - new TableSubquery(node.getLocation().get(), - (Query) process(node.getQuery(), context) - ) - ) - .orElse( - new TableSubquery((Query) process(node.getQuery(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new TableSubquery(node.getLocation().get(), + (Query) process(node.getQuery(), context) + ); + } else { + return new TableSubquery((Query) process(node.getQuery(), context)); + } } protected Node visitAliasedRelation(final AliasedRelation node, final Object context) { - return node.getLocation() - .map(location -> - new AliasedRelation(node.getLocation().get(), - (Relation) process(node.getRelation(), context), - node.getAlias(), - node.getColumnNames()) - ) - .orElse( - new AliasedRelation((Relation) process(node.getRelation(), context), - node.getAlias(), - node.getColumnNames()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new AliasedRelation(node.getLocation().get(), + (Relation) process(node.getRelation(), context), + node.getAlias(), + node.getColumnNames()); + } else { + return new AliasedRelation((Relation) process(node.getRelation(), context), + node.getAlias(), + node.getColumnNames()); + } } protected Node visitJoin(final Join node, final Object context) { //TODO: Will have to look into Criteria later (includes Expression) - return node.getLocation() - .map(location -> - new Join(node.getLocation().get(), - node.getType(), - (Relation) process(node.getLeft(), context), - (Relation) process(node.getRight(), context), - node.getCriteria(), node.getWithinExpression() - ) - ) - .orElse( - new Join(node.getType(), - (Relation) process(node.getLeft(), context), - (Relation) process(node.getRight(), context), - node.getCriteria() - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new Join(node.getLocation().get(), + node.getType(), + (Relation) process(node.getLeft(), context), + (Relation) process(node.getRight(), context), + node.getCriteria(), node.getWithinExpression() + ); + } else { + return new Join(node.getType(), + (Relation) process(node.getLeft(), context), + (Relation) process(node.getRight(), context), + node.getCriteria() + ); + } } protected Node visitExists(final ExistsPredicate node, final Object context) { - return node.getLocation() - .map(location -> - new ExistsPredicate( - node.getLocation().get(), - (Query) process(node.getSubquery(), context) - ) - ) - .orElse( - new ExistsPredicate((Query) process(node.getSubquery(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new ExistsPredicate( + node.getLocation().get(), + (Query) process(node.getSubquery(), context) + ); + } else { + return new ExistsPredicate((Query) process(node.getSubquery(), context)); + } } protected Node visitCast(final Cast node, final Object context) { - return node.getLocation() - .map(location -> - new Cast( - node.getLocation().get(), - (Expression) process(node.getExpression(), context), - node.getType() - ) - ) - .orElse( - new Cast((Expression) process(node.getExpression(), context), - node.getType() - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new Cast( + node.getLocation().get(), + (Expression) process(node.getExpression(), context), + node.getType() + ); + } else { + return new Cast((Expression) process(node.getExpression(), context), + node.getType() + ); + } } protected Node visitFieldReference(final FieldReference node, final Object context) { @@ -842,18 +844,18 @@ protected Node visitFieldReference(final FieldReference node, final Object conte } protected Node visitWindowExpression(final WindowExpression node, final Object context) { - return node.getLocation() - .map(location -> - new WindowExpression( - node.getLocation(), - node.getWindowName(), - (KsqlWindowExpression) process(node.getKsqlWindowExpression(), context)) - ) - .orElse( - new WindowExpression( - node.getWindowName(), - (KsqlWindowExpression) process(node.getKsqlWindowExpression(), context)) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new WindowExpression( + node.getLocation(), + node.getWindowName(), + (KsqlWindowExpression) process(node.getKsqlWindowExpression(), context)); + } else { + return new WindowExpression( + node.getWindowName(), + (KsqlWindowExpression) process(node.getKsqlWindowExpression(), context)); + } } protected Node visitTumblingWindowExpression( @@ -883,73 +885,73 @@ protected Node visitSessionWindowExpression( } protected Node visitWindow(final Window node, final Object context) { - return node.getLocation() - .map(location -> - new Window(node.getLocation().get(), - node.getWindowName(), - (WindowExpression) process(node.getWindowExpression(), context) - ) - ) - .orElse( - new Window(node.getWindowName(), - (WindowExpression) process(node.getWindowExpression(), context) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new Window(node.getLocation().get(), + node.getWindowName(), + (WindowExpression) process(node.getWindowExpression(), context) + ); + } else { + return new Window(node.getWindowName(), + (WindowExpression) process(node.getWindowExpression(), context) + ); + } } protected Node visitWindowFrame(final WindowFrame node, final Object context) { - return node.getLocation() - .map(location -> - new WindowFrame( - node.getLocation().get(), - node.getType(), - (FrameBound) process(node.getStart(), context), - node.getEnd().isPresent() - ? Optional.ofNullable((FrameBound) process(node.getEnd().get(), context)) - : Optional.empty() - ) - ) - .orElse( - new WindowFrame( - node.getType(), - (FrameBound) process(node.getStart(), context), - node.getEnd().isPresent() - ? Optional.ofNullable((FrameBound) process(node.getEnd().get(), context)) - : Optional.empty() - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new WindowFrame( + node.getLocation().get(), + node.getType(), + (FrameBound) process(node.getStart(), context), + node.getEnd().isPresent() + ? Optional.ofNullable((FrameBound) process(node.getEnd().get(), context)) + : Optional.empty() + ); + } else { + return new WindowFrame( + node.getType(), + (FrameBound) process(node.getStart(), context), + node.getEnd().isPresent() + ? Optional.ofNullable((FrameBound) process(node.getEnd().get(), context)) + : Optional.empty() + ); + } } protected Node visitFrameBound(final FrameBound node, final Object context) { - return node.getLocation() - .map(location -> - new FrameBound(node.getLocation().get(), - node.getType(), - node.getValue().isPresent() - ? (Expression) process(node.getValue().get(), context) - : null - ) - ) - .orElse( - new FrameBound(node.getType(), - node.getValue().isPresent() - ? (Expression) process(node.getValue().get(), context) - : null - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new FrameBound(node.getLocation().get(), + node.getType(), + node.getValue().isPresent() + ? (Expression) process(node.getValue().get(), context) + : null + ); + } else { + return new FrameBound(node.getType(), + node.getValue().isPresent() + ? (Expression) process(node.getValue().get(), context) + : null + ); + } } protected Node visitTableElement(final TableElement node, final Object context) { - return node.getLocation() - .map(location -> - new TableElement(node.getLocation().get(), - node.getName(), - node.getType()) - ) - .orElse( - new TableElement(node.getName(), - node.getType()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new TableElement(node.getLocation().get(), + node.getName(), + node.getType()); + } else { + return new TableElement(node.getName(), + node.getType()); + } } protected Node visitCreateStream(final CreateStream node, final Object context) { @@ -1032,91 +1034,90 @@ protected Node visitDropTable(final DropTable node, final Object context) { } protected Node visitRenameColumn(final RenameColumn node, final Object context) { - return node.getLocation() - .map(location -> - new RenameColumn(node.getLocation().get(), - node.getTable(), - node.getSource(), - node.getTarget() - ) - ) - .orElse( - new RenameColumn(node.getTable(), - node.getSource(), - node.getTarget() - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new RenameColumn(node.getLocation().get(), + node.getTable(), + node.getSource(), + node.getTarget() + ); + } else { + return new RenameColumn(node.getTable(), + node.getSource(), + node.getTarget() + ); + } } protected Node visitDropView(final DropView node, final Object context) { - return node.getLocation() - .map(location -> - new DropView(node.getLocation().get(), - node.getName(), - node.isExists()) - ) - .orElse( - new DropView(node.getName(), - node.isExists()) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new DropView(node.getLocation().get(), + node.getName(), + node.isExists()); + } else { + return new DropView(node.getName(), + node.isExists()); + } } protected Node visitDelete(final Delete node, final Object context) { - return node.getLocation() - .map(location -> - new Delete(node.getLocation().get(), - (Table) process(node.getTable(), context), - node.getWhere().isPresent() - ? Optional.ofNullable((Expression) process(node.getWhere().get(), context)) - : Optional.empty() - ) - ) - .orElse( - new Delete((Table) process(node.getTable(), context), - node.getWhere().isPresent() - ? Optional.ofNullable((Expression) process(node.getWhere().get(), context)) - : Optional.empty() - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new Delete(node.getLocation().get(), + (Table) process(node.getTable(), context), + node.getWhere().isPresent() + ? Optional.ofNullable((Expression) process(node.getWhere().get(), context)) + : Optional.empty() + ); + } else { + return new Delete((Table) process(node.getTable(), context), + node.getWhere().isPresent() + ? Optional.ofNullable((Expression) process(node.getWhere().get(), context)) + : Optional.empty() + ); + } } protected Node visitGroupBy(final GroupBy node, final Object context) { - return node.getLocation() - .map(location -> - new GroupBy( - node.getLocation().get(), - node.isDistinct(), - node.getGroupingElements().stream() - .map(groupingElement -> (GroupingElement) process(groupingElement, context)) - .collect(Collectors.toList()) - ) - ) - .orElse( - new GroupBy( - node.isDistinct(), - node.getGroupingElements().stream() - .map(groupingElement -> (GroupingElement) process(groupingElement, context)) - .collect(Collectors.toList()) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new GroupBy( + node.getLocation().get(), + node.isDistinct(), + node.getGroupingElements().stream() + .map(groupingElement -> (GroupingElement) process(groupingElement, context)) + .collect(Collectors.toList())); + } else { + return new GroupBy( + node.isDistinct(), + node.getGroupingElements().stream() + .map(groupingElement -> (GroupingElement) process(groupingElement, context)) + .collect(Collectors.toList()) + ); + } } protected Node visitSimpleGroupBy(final SimpleGroupBy node, final Object context) { - return node.getLocation() - .map(location -> - new SimpleGroupBy(node.getLocation().get(), - node.getColumnExpressions().stream() - .map(ce -> (Expression) process(ce, context)) - .collect(Collectors.toList()) - ) - ) - .orElse( - new SimpleGroupBy(node.getColumnExpressions().stream() - .map(ce -> (Expression) process(ce, context)) - .collect(Collectors.toList()) - ) - ); + // use an if/else block here (instead of isPresent.map(...).orElse(...)) so only one object + // gets instantiated (issue #1784) + if (node.getLocation().isPresent()) { + return new SimpleGroupBy(node.getLocation().get(), + node.getColumnExpressions().stream() + .map(ce -> (Expression) process(ce, context)) + .collect(Collectors.toList()) + ); + } else { + return new SimpleGroupBy(node.getColumnExpressions().stream() + .map(ce -> (Expression) process(ce, context)) + .collect(Collectors.toList()) + ); + } } protected Node visitSymbolReference(final SymbolReference node, final Object context) {