Bugfix, copy of AggregationOperator should be same

Signed-off-by: penghuo <penghuo@gmail.com>
opensearch-project · Sep 6, 2022 · 6137880 · 6137880
1 parent 4e40ed2
commit 6137880
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 3 deletions.
diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/AggregationOperator.java b/core/src/main/java/org/opensearch/sql/planner/physical/AggregationOperator.java
@@ -55,14 +55,19 @@ public AggregationOperator(PhysicalPlan input, List<NamedAggregator> aggregatorL
                              List<NamedExpression> groupByExprList) {
     this.input = input;
     this.aggregatorList = aggregatorList;
+    this.groupByExprList = groupByExprList;
     if (hasSpan(groupByExprList)) {
+      // span expression is always the first expression in group list if exist.
       this.span = groupByExprList.get(0);
-      this.groupByExprList = groupByExprList.subList(1, groupByExprList.size());
+      this.collector =
+          Collector.Builder.build(
+              this.span, groupByExprList.subList(1, groupByExprList.size()), this.aggregatorList);
+
     } else {
       this.span = null;
-      this.groupByExprList = groupByExprList;
+      this.collector =
+          Collector.Builder.build(this.span, this.groupByExprList, this.aggregatorList);
     }
-    this.collector = Collector.Builder.build(this.span, this.groupByExprList, this.aggregatorList);
   }
 
   @Override

diff --git a/core/src/test/java/org/opensearch/sql/planner/physical/AggregationOperatorTest.java b/core/src/test/java/org/opensearch/sql/planner/physical/AggregationOperatorTest.java
@@ -495,4 +495,17 @@ public void twoBucketsSpanAndLong() {
             "span", new ExprDateValue("2021-01-07"), "region","iad", "host", "h2", "max", 8))
     ));
   }
+
+  @Test
+  public void copyOfAggregationOperatorShouldSame() {
+    AggregationOperator plan = new AggregationOperator(testScan(datetimeInputs),
+        Collections.singletonList(DSL
+            .named("count", dsl.count(DSL.ref("second", TIMESTAMP)))),
+        Collections.singletonList(DSL
+            .named("span", DSL.span(DSL.ref("second", TIMESTAMP), DSL.literal(6 * 1000), "ms"))));
+    AggregationOperator copy = new AggregationOperator(plan.getInput(), plan.getAggregatorList(),
+        plan.getGroupByExprList());
+
+    assertEquals(plan, copy);
+  }
 }