Skip to content

Commit

Permalink
fix coverage
Browse files Browse the repository at this point in the history 
Signed-off-by: Lantao Jin <ltjin@amazon.com>
  • Loading branch information
@LantaoJin
LantaoJin committed Jul 3, 2024
1 parent 81f913c commit 23da5b0
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 103 deletions.
1 change: 1 addition & 0 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ spotless {
removeUnusedImports()
trimTrailingWhitespace()
endWithNewline()
toggleOffOn()
googleJavaFormat('1.17.0').reflowLongStrings().groupArtifact('com.google.googlejavaformat:google-java-format')
}
}
Expand Down
1 change: 1 addition & 0 deletions core/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ spotless {
removeUnusedImports()
trimTrailingWhitespace()
endWithNewline()
toggleOffOn()
googleJavaFormat('1.17.0').reflowLongStrings().groupArtifact('com.google.googlejavaformat:google-java-format')
}
}
Expand Down
45 changes: 23 additions & 22 deletions core/src/main/java/org/opensearch/sql/analysis/Analyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,26 +342,27 @@ public LogicalPlan visitAggregation(Aggregation node, AnalysisContext context) {
// +------+------------------------------------------------------------------------------------------+---------+-------------------------+
// | Case | Query | IsValid | Field Missed In GroupBy |
// +------+------------------------------------------------------------------------------------------+---------+-------------------------+
// | 0 | SELECT a FROM table GROUP BY a | Yes | N/A |
// | 1 | SELECT a FROM table GROUP BY a * 3 | No | a |
// | 2 | SELECT a * 3 FROM table GROUP BY a | Yes | N/A |
// | 3 | SELECT a * 3 FROM table GROUP BY b | No | a |
// | 4 | SELECT a FROM table GROUP BY upper(a) | No | a |
// | 5 | SELECT upper(a) FROM table GROUP BY a | Yes | N/A |
// | 6 | SELECT upper(a) FROM table GROUP BY upper(a) | Yes | N/A |
// | 7 | SELECT concat(upper(a), upper(b)) FROM table GROUP BY b | No | a |
// | 8 | SELECT concat(upper(a), upper(b)) FROM table GROUP BY upper(b) | No | a |
// | 9 | SELECT concat(upper(a), upper(b)) FROM table GROUP BY concat(upper(a), upper(b)) | Yes | N/A |
// | 10 | SELECT concat(upper(a), upper(b)) FROM table GROUP BY concat_ws(',', upper(a), upper(b)) | No | a |
// | 11 | SELECT concat(a, b) FROM test_bulk1 group by upper(a), upper(b) | No | a |
// | 12 | SELECT concat(a, b) FROM test_bulk1 group by a | No | b |
// | 13 | SELECT concat(a, b) FROM test_bulk1 group by a, upper(b) | No | b |
// | 14 | SELECT concat(a, b), upper(b) FROM test_bulk1 group by a, upper(b) | No | b |
// | 15 | SELECT concat(a, b), upper(b) FROM test_bulk1 group by a, b | Yes | N/A |
// | 16 | SELECT upper(concat(a, b)) FROM test_bulk1 group by concat(a, b) | Yes | N/A |
// | 17 | SELECT concat(concat(a, b), c) FROM test_bulk1 group by concat(a, b) | No | c |
// | 18 | SELECT 1, 2, 3 FROM test_bulk1 group by a | Yes | N/A |
// | 19 | SELECT 1, 2, b FROM test_bulk1 group by a | No | b |
// | 1 | SELECT a FROM table GROUP BY b | No | a |
// | 2 | SELECT a as c FROM table GROUP BY b | No | a |
// | 3 | SELECT a FROM table GROUP BY a * 3 | No | a |
// | 4 | SELECT a * 3 FROM table GROUP BY a | Yes | N/A |
// | 5 | SELECT a * 3 FROM table GROUP BY b | No | a |
// | 6 | SELECT a FROM table GROUP BY upper(a) | No | a |
// | 7 | SELECT upper(a) FROM table GROUP BY a | Yes | N/A |
// | 8 | SELECT upper(a) FROM table GROUP BY upper(a) | Yes | N/A |
// | 9 | SELECT concat(upper(a), upper(b)) FROM table GROUP BY b | No | a |
// | 10 | SELECT concat(upper(a), upper(b)) FROM table GROUP BY upper(b) | No | a |
// | 11 | SELECT concat(upper(a), upper(b)) FROM table GROUP BY concat(upper(a), upper(b)) | Yes | N/A |
// | 12 | SELECT concat(upper(a), upper(b)) FROM table GROUP BY concat_ws(',', upper(a), upper(b)) | No | a |
// | 13 | SELECT concat(a, b) FROM table group by upper(a), upper(b) | No | a |
// | 14 | SELECT concat(a, b) FROM table group by a | No | b |
// | 15 | SELECT concat(a, b) FROM table group by a, upper(b) | No | b |
// | 16 | SELECT concat(a, b), upper(b) FROM table group by a, upper(b) | No | b |
// | 17 | SELECT concat(a, b), upper(b) FROM table group by a, b | Yes | N/A |
// | 18 | SELECT upper(concat(a, b)) FROM table group by concat(a, b) | Yes | N/A |
// | 19 | SELECT concat(concat(a, b), c) FROM table group by concat(a, b) | No | c |
// | 20 | SELECT 1, 2, 3 FROM table group by a | Yes | N/A |
// | 21 | SELECT 1, 2, b FROM table group by a | No | b |
// +------+------------------------------------------------------------------------------------------+---------+-------------------------+
// spotless:on
for (UnresolvedExpression expr : node.getAliasFreeSelectExprList()) {
Expand All @@ -370,11 +371,11 @@ public LogicalPlan visitAggregation(Aggregation node, AnalysisContext context) {
e ->
(e instanceof ReferenceExpression || e instanceof FunctionExpression)
&& groupBys.stream().noneMatch(g -> e.equals(g.getDelegated()));
Consumer<String> exceptionConsumer =
Consumer<String> action =
name -> {
throw QueryCompilationError.fieldNotInGroupByClauseError(name);
};
ExpressionUtils.invalidateExpression(resolvedSelectItemExpr, notExists, exceptionConsumer);
ExpressionUtils.actionOnCheck(resolvedSelectItemExpr, notExists, action);
}

// resolve aggregators
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/opensearch/sql/executor/pagination/CanPaginateVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public Boolean visitField(Field node, Object context) {

@Override
public Boolean visitAlias(Alias node, Object context) {
return canPaginate(node, context) && node.getDelegated().accept(this, context);
return canPaginate(node, context);
}

@Override
Expand Down
17 changes: 11 additions & 6 deletions core/src/main/java/org/opensearch/sql/utils/ExpressionUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import lombok.Generated;
import lombok.experimental.UtilityClass;
import org.opensearch.sql.expression.Expression;
import org.opensearch.sql.expression.FunctionExpression;
Expand Down Expand Up @@ -54,23 +55,27 @@ private static <T extends Expression> void findSubExpressionsHelper(
}

/**
* Traverse an {@link Expression} to throw runtime exception when the given predicate matched.
* Traverse an {@link Expression} to consume when the given predicate matched.
*
* <p>Add @Generated annotation since the jacoco has a bug to report test coverage for the Lambda
* {@code action.accept()}.
*
* @param expr the expression to traverse
* @param condition the condition to test
* @param exception the exception to throw when the condition matched.
* @param action execute the action when the condition matched.
*/
public static <T extends Expression> void invalidateExpression(
Expression expr, Predicate<Expression> condition, Consumer<String> exception) {
@Generated
public static <T extends Expression> void actionOnCheck(
Expression expr, Predicate<Expression> condition, Consumer<String> action) {
if (expr instanceof FunctionExpression) {
if (!condition.test(expr)) {
return;
}
for (Expression child : ((FunctionExpression) expr).getArguments()) {
invalidateExpression(child, condition, exception);
actionOnCheck(child, condition, action);
}
} else if (condition.test(expr)) {
exception.accept(expr.toString());
action.accept(expr.toString());
}
}
}
Loading

0 comments on commit 23da5b0

Please sign in to comment.