From 4de9272ae235aa89bd335cec650132d6ba6c90b5 Mon Sep 17 00:00:00 2001 From: GabeFernandez310 Date: Mon, 6 Mar 2023 10:46:50 -0800 Subject: [PATCH 1/2] Added Fix Signed-off-by: GabeFernandez310 --- .../sql/planner/physical/ProjectOperator.java | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java b/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java index 496e4e6ddb..d96dd22ae0 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java @@ -9,8 +9,11 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -56,33 +59,56 @@ public boolean hasNext() { public ExprValue next() { ExprValue inputValue = input.next(); ImmutableMap.Builder mapBuilder = new Builder<>(); + Set columns = new HashSet<>(); // ParseExpression will always override NamedExpression when identifier conflicts // TODO needs a better implementation, see https://github.com/opensearch-project/sql/issues/458 + // TODO: Implement a fallback: if a key exists, append a number at the end for (NamedExpression expr : projectList) { ExprValue exprValue = expr.valueOf(inputValue.bindingTuples()); Optional optionalParseExpression = namedParseExpressions.stream() .filter(parseExpr -> parseExpr.getNameOrAlias().equals(expr.getNameOrAlias())) .findFirst(); + String columnName = expr.getNameOrAlias(); + if (optionalParseExpression.isEmpty()) { - mapBuilder.put(expr.getNameOrAlias(), exprValue); + + if (columns.contains(expr.getNameOrAlias())) { + int appendedNum = 1; + while (columns.contains(String.format("%s%d", expr.getNameOrAlias(), appendedNum))) { + appendedNum++; + } + columnName = String.format("%s%d", expr.getNameOrAlias(), appendedNum); + } + columns.add(columnName); + mapBuilder.put(columnName, exprValue); continue; } NamedExpression parseExpression = optionalParseExpression.get(); ExprValue sourceFieldValue = inputValue.bindingTuples() .resolve(((ParseExpression) parseExpression.getDelegated()).getSourceField()); + Set columns2 = new HashSet<>(); + if (columns2.contains(parseExpression.getNameOrAlias())) { + int appendedNum = 1; + while (columns.contains(String.format("%s%d", parseExpression.getNameOrAlias(), appendedNum))) { + appendedNum++; + } + columnName = String.format("%s%d", parseExpression.getNameOrAlias(), appendedNum); + } + columns2.add(columnName); + if (sourceFieldValue.isMissing()) { // source field will be missing after stats command, read from inputValue if it exists // otherwise do nothing since it should not appear as a field ExprValue tupleValue = - ExprValueUtils.getTupleValue(inputValue).get(parseExpression.getNameOrAlias()); + ExprValueUtils.getTupleValue(inputValue).get(columnName); if (tupleValue != null) { - mapBuilder.put(parseExpression.getNameOrAlias(), tupleValue); + mapBuilder.put(columnName, tupleValue); } } else { ExprValue parsedValue = parseExpression.valueOf(inputValue.bindingTuples()); - mapBuilder.put(parseExpression.getNameOrAlias(), parsedValue); + mapBuilder.put(columnName, parsedValue); } } return ExprTupleValue.fromExprValueMap(mapBuilder.build()); From c00ae9498fe997bee5a78d0ea75a21798d29be75 Mon Sep 17 00:00:00 2001 From: GabeFernandez310 Date: Mon, 6 Mar 2023 11:32:43 -0800 Subject: [PATCH 2/2] Cleaned Implementation Signed-off-by: GabeFernandez310 --- .../sql/planner/physical/ProjectOperator.java | 37 +++++++------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java b/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java index d96dd22ae0..adc05108eb 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java @@ -11,7 +11,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -55,6 +54,18 @@ public boolean hasNext() { return input.hasNext(); } + private String handleDuplicateColumns(String newName, Set takenNames) { + if (takenNames.contains(newName)) { + int appendedNum = 1; + while (takenNames.contains(String.format("%s%d", newName, appendedNum))) { + appendedNum++; + } + newName = String.format("%s%d", newName, appendedNum); + } + takenNames.add(newName); + return newName; + } + @Override public ExprValue next() { ExprValue inputValue = input.next(); @@ -63,41 +74,21 @@ public ExprValue next() { // ParseExpression will always override NamedExpression when identifier conflicts // TODO needs a better implementation, see https://github.com/opensearch-project/sql/issues/458 - // TODO: Implement a fallback: if a key exists, append a number at the end for (NamedExpression expr : projectList) { ExprValue exprValue = expr.valueOf(inputValue.bindingTuples()); Optional optionalParseExpression = namedParseExpressions.stream() .filter(parseExpr -> parseExpr.getNameOrAlias().equals(expr.getNameOrAlias())) .findFirst(); - String columnName = expr.getNameOrAlias(); if (optionalParseExpression.isEmpty()) { - - if (columns.contains(expr.getNameOrAlias())) { - int appendedNum = 1; - while (columns.contains(String.format("%s%d", expr.getNameOrAlias(), appendedNum))) { - appendedNum++; - } - columnName = String.format("%s%d", expr.getNameOrAlias(), appendedNum); - } - columns.add(columnName); - mapBuilder.put(columnName, exprValue); + mapBuilder.put(handleDuplicateColumns(expr.getNameOrAlias(), columns), exprValue); continue; } NamedExpression parseExpression = optionalParseExpression.get(); ExprValue sourceFieldValue = inputValue.bindingTuples() .resolve(((ParseExpression) parseExpression.getDelegated()).getSourceField()); - Set columns2 = new HashSet<>(); - if (columns2.contains(parseExpression.getNameOrAlias())) { - int appendedNum = 1; - while (columns.contains(String.format("%s%d", parseExpression.getNameOrAlias(), appendedNum))) { - appendedNum++; - } - columnName = String.format("%s%d", parseExpression.getNameOrAlias(), appendedNum); - } - columns2.add(columnName); - + String columnName = handleDuplicateColumns(parseExpression.getNameOrAlias(), columns); if (sourceFieldValue.isMissing()) { // source field will be missing after stats command, read from inputValue if it exists // otherwise do nothing since it should not appear as a field