Skip to content

Commit 3981c38

Browse files
committed
Minor cleanup.
Signed-off-by: currantw <[email protected]>
1 parent 237b69e commit 3981c38

File tree

10 files changed

+32
-41
lines changed

10 files changed

+32
-41
lines changed

core/src/main/java/org/opensearch/sql/analysis/Analyzer.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public class Analyzer extends AbstractNodeVisitor<LogicalPlan, AnalysisContext>
134134
private final BuiltinFunctionRepository repository;
135135

136136
private static final String PATH_SEPARATOR = ".";
137-
private static final Pattern PATH_COMPONENT_PATTERN =
137+
private static final Pattern PATH_SEPARATOR_PATTERN =
138138
Pattern.compile(PATH_SEPARATOR, Pattern.LITERAL);
139139

140140
/** Constructor. */
@@ -491,7 +491,7 @@ public LogicalPlan visitFlatten(Flatten node, AnalysisContext context) {
491491

492492
for (java.util.Map.Entry<String, ExprType> entry : fieldsMap.entrySet()) {
493493
String path = entry.getKey();
494-
List<String> pathComponents = Arrays.stream(PATH_COMPONENT_PATTERN.split(path)).toList();
494+
List<String> pathComponents = Arrays.stream(PATH_SEPARATOR_PATTERN.split(path)).toList();
495495

496496
// Verify that path starts with the field name.
497497
if (!pathComponents.getFirst().equals(fieldName)) {

core/src/main/java/org/opensearch/sql/planner/DefaultImplementor.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public PhysicalPlan visitEval(LogicalEval node, C context) {
103103

104104
@Override
105105
public PhysicalPlan visitFlatten(LogicalFlatten node, C context) {
106-
return new FlattenOperator(visitChild(node, context), node.getField());
106+
return new FlattenOperator(visitChild(node, context), node.getFieldRefExp());
107107
}
108108

109109
@Override

core/src/main/java/org/opensearch/sql/planner/logical/LogicalFlatten.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@
1616
@ToString
1717
@EqualsAndHashCode(callSuper = true)
1818
public class LogicalFlatten extends LogicalPlan {
19-
private final ReferenceExpression field;
19+
private final ReferenceExpression fieldRefExp;
2020

21-
public LogicalFlatten(LogicalPlan child, ReferenceExpression field) {
21+
public LogicalFlatten(LogicalPlan child, ReferenceExpression fieldRefExp) {
2222
super(Collections.singletonList(child));
23-
this.field = field;
23+
this.fieldRefExp = fieldRefExp;
2424
}
2525

2626
@Override

core/src/main/java/org/opensearch/sql/planner/logical/LogicalPlanDSL.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ public static LogicalPlan eval(
9797
return new LogicalEval(input, Arrays.asList(expressions));
9898
}
9999

100-
public LogicalPlan flatten(LogicalPlan input, ReferenceExpression field) {
101-
return new LogicalFlatten(input, field);
100+
public LogicalPlan flatten(LogicalPlan input, ReferenceExpression fieldRefExp) {
101+
return new LogicalFlatten(input, fieldRefExp);
102102
}
103103

104104
public static LogicalPlan sort(LogicalPlan input, Pair<SortOption, Expression>... sorts) {

core/src/main/java/org/opensearch/sql/planner/physical/FlattenOperator.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public ExprValue next() {
5353
ExprValue inputExprValue = input.next();
5454
Map<String, ExprValue> fieldsMap = ExprValueUtils.getTupleValue(inputExprValue);
5555

56-
// Get the flattened field map.
56+
// Build the flattened field map.
5757
String fieldName = field.getAttr();
5858
ExprValue exprValue = fieldsMap.get(fieldName);
5959

@@ -63,7 +63,7 @@ public ExprValue next() {
6363
fieldsMap.putAll(flattenedFieldsMap);
6464
fieldsMap.remove(fieldName);
6565

66-
// Update environment.
66+
// Update the environment.
6767
Environment<Expression, ExprValue> env = inputExprValue.bindingTuples();
6868

6969
for (Entry<String, ExprValue> entry : flattenedFieldsMap.entrySet()) {

core/src/main/java/org/opensearch/sql/planner/physical/PhysicalPlanDSL.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ public static EvalOperator eval(
6060
return new EvalOperator(input, Arrays.asList(expressions));
6161
}
6262

63-
public FlattenOperator flatten(PhysicalPlan input, ReferenceExpression field) {
64-
return new FlattenOperator(input, field);
63+
public FlattenOperator flatten(PhysicalPlan input, ReferenceExpression fieldRefExp) {
64+
return new FlattenOperator(input, fieldRefExp);
6565
}
6666

6767
public static SortOperator sort(PhysicalPlan input, Pair<SortOption, Expression>... sorts) {

core/src/test/java/org/opensearch/sql/planner/DefaultImplementorTest.java

+3-8
Original file line numberDiff line numberDiff line change
@@ -342,13 +342,8 @@ void visitFlatten_should_build_FlattenOperator() {
342342
var logicalPlan = new LogicalFlatten(logicalChild, ref(fieldName, STRUCT));
343343
var implemented = logicalPlan.accept(implementor, null);
344344

345-
assertInstanceOf(
346-
FlattenOperator.class, implemented, "Visiting logical flatten builds physical flatten");
347-
assertEquals(
348-
fieldName,
349-
((FlattenOperator) implemented).getField().getAttr(),
350-
"Physical flatten has expected field");
351-
assertSame(
352-
physicalChild, implemented.getChild().getFirst(), "Physical flatten has expected child");
345+
assertInstanceOf(FlattenOperator.class, implemented);
346+
assertEquals(fieldName, ((FlattenOperator) implemented).getField().getAttr());
347+
assertSame(physicalChild, implemented.getChild().getFirst());
353348
}
354349
}

core/src/test/java/org/opensearch/sql/planner/physical/FlattenOperatorTest.java

+14-14
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,20 @@
3333
class FlattenOperatorTest extends PhysicalPlanTestBase {
3434
@Mock private PhysicalPlan inputPlan;
3535

36+
@Test
37+
void testFlattenStructEmpty() {
38+
Map<String, Object> structMap = ImmutableMap.of();
39+
Map<String, Object> rowMap = ImmutableMap.of("struct_field", structMap);
40+
ExprValue rowValue = ExprValueUtils.tupleValue(rowMap);
41+
42+
when(inputPlan.hasNext()).thenReturn(true, false);
43+
when(inputPlan.next()).thenReturn(rowValue);
44+
45+
PhysicalPlan plan = flatten(inputPlan, DSL.ref("struct_field", STRUCT));
46+
47+
assertThat(execute(plan), allOf(iterableWithSize(1), hasItems()));
48+
}
49+
3650
@Test
3751
void testFlattenStructBasic() {
3852
Map<String, Object> structMap =
@@ -56,20 +70,6 @@ void testFlattenStructBasic() {
5670
assertThat(execute(plan), allOf(iterableWithSize(1), hasItems(expectedRowValue)));
5771
}
5872

59-
@Test
60-
void testFlattenStructEmpty() {
61-
Map<String, Object> structMap = ImmutableMap.of();
62-
Map<String, Object> rowMap = ImmutableMap.of("struct_field", structMap);
63-
ExprValue rowValue = ExprValueUtils.tupleValue(rowMap);
64-
65-
when(inputPlan.hasNext()).thenReturn(true, false);
66-
when(inputPlan.next()).thenReturn(rowValue);
67-
68-
PhysicalPlan plan = flatten(inputPlan, DSL.ref("struct_field", STRUCT));
69-
70-
assertThat(execute(plan), allOf(iterableWithSize(1), hasItems()));
71-
}
72-
7373
@Test
7474
void testFlattenStructNested() {
7575
Map<String, Object> nestedStructMap =

opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -369,10 +369,9 @@ void test_visitOpenSearchEval() {
369369
void test_visitFlatten() {
370370
FlattenOperator flattenOperator =
371371
new FlattenOperator(values(emptyList()), ref("field_name", STRUCT));
372+
372373
assertEquals(
373-
resourceMonitor(flattenOperator),
374-
executionProtector.visitFlatten(flattenOperator, null),
375-
"flatten operator is protected");
374+
resourceMonitor(flattenOperator), executionProtector.visitFlatten(flattenOperator, null));
376375
}
377376

378377
PhysicalPlan resourceMonitor(PhysicalPlan input) {

ppl/src/test/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java

+1-4
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,7 @@ public void testTrendlineCommand() {
9898

9999
@Test
100100
public void testFlattenCommand() {
101-
assertEquals(
102-
"Flatten command is not modified by anonymizer",
103-
"source=t | flatten field_name",
104-
anonymize("source=t | flatten field_name"));
101+
assertEquals("source=t | flatten field_name", anonymize("source=t | flatten field_name"));
105102
}
106103

107104
@Test

0 commit comments

Comments
 (0)