Copilot commented on code in PR #16761:
URL: https://github.com/apache/iotdb/pull/16761#discussion_r2570966006


##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/MergeExceptTest.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.relational.analyzer;
+
+import org.apache.iotdb.db.queryengine.plan.relational.planner.PlanTester;
+
+import org.junit.Test;
+
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanAssert.assertPlan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.aggregation;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.filter;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.output;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.project;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.sort;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.tableScan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.union;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.window;
+
+/**
+ * for except node, only merge first source node
+ *
+ * <p>For parent and child of type EXCEPT: 1. if parent is EXCEPT DISTINCT and 
child is EXCEPT ALL,
+ * merge is not possible 2. if parent and child are both EXCEPT DISTINCT, the 
resulting set
+ * operation is EXCEPT DISTINCT 3. if parent and child are both EXCEPT ALL, 
the resulting set
+ * operation is EXCEPT ALL 4. if parent is EXCEPT ALL and child is EXCEPT 
DISTINCT, the resulting
+ * set operation is EXCEPT DISTINCT
+ */
+public class MergeExceptTest {
+
+  @Test
+  public void MergeTwoExceptAll() {

Review Comment:
   Test method name should follow Java naming conventions. Method names should 
start with a lowercase letter. Consider renaming `MergeTwoExceptAll()` to 
`mergeTwoExceptAll()`.
   ```suggestion
     public void mergeTwoExceptAll() {
   ```



##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/MergeExceptTest.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.relational.analyzer;
+
+import org.apache.iotdb.db.queryengine.plan.relational.planner.PlanTester;
+
+import org.junit.Test;
+
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanAssert.assertPlan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.aggregation;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.filter;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.output;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.project;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.sort;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.tableScan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.union;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.window;
+
+/**
+ * for except node, only merge first source node
+ *
+ * <p>For parent and child of type EXCEPT: 1. if parent is EXCEPT DISTINCT and 
child is EXCEPT ALL,
+ * merge is not possible 2. if parent and child are both EXCEPT DISTINCT, the 
resulting set
+ * operation is EXCEPT DISTINCT 3. if parent and child are both EXCEPT ALL, 
the resulting set
+ * operation is EXCEPT ALL 4. if parent is EXCEPT ALL and child is EXCEPT 
DISTINCT, the resulting
+ * set operation is EXCEPT DISTINCT
+ */
+public class MergeExceptTest {
+
+  @Test
+  public void MergeTwoExceptAll() {
+    PlanTester planTester = new PlanTester();
+
+    // the parent
+    assertPlan(
+        planTester.createPlan(
+            "select tag1 from t1 except all select tag1 from t2 except all 
select tag1 from t3 "),
+        output(
+            project(
+                filter(
+                    project(
+                        window(
+                            sort(
+                                union(
+                                    project(tableScan("testdb.t1")),
+                                    project(tableScan("testdb.t2")),
+                                    project(tableScan("testdb.t3"))))))))));
+  }
+
+  @Test
+  public void MergeTwoExceptDistinct() {

Review Comment:
   Test method name should follow Java naming conventions. Method names should 
start with a lowercase letter. Consider renaming `MergeTwoExceptDistinct()` to 
`mergeTwoExceptDistinct()`.
   ```suggestion
     public void mergeTwoExceptDistinct() {
   ```



##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/MergeIntersectTest.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.relational.analyzer;
+
+import org.apache.iotdb.db.queryengine.plan.relational.planner.PlanTester;
+
+import org.junit.Test;
+
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanAssert.assertPlan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.aggregation;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.filter;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.output;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.project;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.sort;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.tableScan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.union;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.window;
+
+/**
+ * For parent and child of type INTERSECT, merge is always possible: 1. if 
parent and child are both
+ * INTERSECT ALL, the resulting set operation is INTERSECT ALL 2. otherwise, 
the resulting set
+ * operation is INTERSECT DISTINCT: 3. if the parent is DISTINCT, the result 
has unique values,
+ * regardless of whether child branches were DISTINCT or ALL, 4. if the child 
is DISTINCT, that
+ * branch is guaranteed to have unique values, so at most one element of the 
other branches will be
+ * retained -- this is equivalent to just doing DISTINCT on the parent.

Review Comment:
   The class-level documentation has inconsistent numbering and formatting. 
Item 2 says "otherwise, the resulting set operation is INTERSECT DISTINCT:" but 
then continues with items 3 and 4 that explain sub-conditions. Consider 
reformatting as: "2. otherwise, the resulting set operation is INTERSECT 
DISTINCT because: (a) if the parent is DISTINCT... (b) if the child is 
DISTINCT..."
   ```suggestion
    * For parent and child of type INTERSECT, merge is always possible:
    * 1. If parent and child are both INTERSECT ALL, the resulting set 
operation is INTERSECT ALL.
    * 2. Otherwise, the resulting set operation is INTERSECT DISTINCT because:
    *    (a) If the parent is DISTINCT, the result has unique values, 
regardless of whether child branches were DISTINCT or ALL.
    *    (b) If the child is DISTINCT, that branch is guaranteed to have unique 
values, so at most one element of the other branches will be retained—this is 
equivalent to just doing DISTINCT on the parent.
   ```



##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/MergeExceptTest.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.relational.analyzer;
+
+import org.apache.iotdb.db.queryengine.plan.relational.planner.PlanTester;
+
+import org.junit.Test;
+
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanAssert.assertPlan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.aggregation;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.filter;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.output;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.project;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.sort;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.tableScan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.union;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.window;
+
+/**
+ * for except node, only merge first source node
+ *
+ * <p>For parent and child of type EXCEPT: 1. if parent is EXCEPT DISTINCT and 
child is EXCEPT ALL,
+ * merge is not possible 2. if parent and child are both EXCEPT DISTINCT, the 
resulting set
+ * operation is EXCEPT DISTINCT 3. if parent and child are both EXCEPT ALL, 
the resulting set
+ * operation is EXCEPT ALL 4. if parent is EXCEPT ALL and child is EXCEPT 
DISTINCT, the resulting
+ * set operation is EXCEPT DISTINCT
+ */
+public class MergeExceptTest {
+
+  @Test
+  public void MergeTwoExceptAll() {
+    PlanTester planTester = new PlanTester();
+
+    // the parent
+    assertPlan(
+        planTester.createPlan(
+            "select tag1 from t1 except all select tag1 from t2 except all 
select tag1 from t3 "),
+        output(
+            project(
+                filter(
+                    project(
+                        window(
+                            sort(
+                                union(
+                                    project(tableScan("testdb.t1")),
+                                    project(tableScan("testdb.t2")),
+                                    project(tableScan("testdb.t3"))))))))));
+  }
+
+  @Test
+  public void MergeTwoExceptDistinct() {
+    PlanTester planTester = new PlanTester();
+
+    // the parent
+    assertPlan(
+        planTester.createPlan(
+            "select tag1 from t1 except distinct select tag1 from t2 except 
distinct select tag1 from t3 "),
+        output(
+            project(
+                filter(
+                    aggregation(
+                        union(
+                            project(tableScan("testdb.t1")),
+                            project(tableScan("testdb.t2")),
+                            project(tableScan("testdb.t3"))))))));
+  }
+
+  @Test
+  public void MergeExceptAllAndExceptDistinct() {

Review Comment:
   Test method name should follow Java naming conventions. Method names should 
start with a lowercase letter. Consider renaming 
`MergeExceptAllAndExceptDistinct()` to `mergeExceptAllAndExceptDistinct()`.
   ```suggestion
     public void mergeExceptAllAndExceptDistinct() {
   ```



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/iterative/rule/SetOperationMerge.java:
##########
@@ -138,7 +207,37 @@ private Optional<Boolean> 
mergedQuantifierIsDistinct(SetOperationNode node, Plan
       return Optional.of(false);
     }
 
-    // the Judgment logic for intersect and except wait for supplying
-    return Optional.empty();
+    if (node instanceof IntersectNode) {
+      if (!((IntersectNode) node).isDistinct() && !((IntersectNode) 
child).isDistinct()) {
+        return Optional.of(false);
+      }
+      return Optional.of(true);
+    }
+
+    // never should reach here
+    throw new IllegalStateException(
+        "unexpected setOperation node type: " + 
node.getClass().getSimpleName());
+  }
+
+  /**
+   * Check if node and child are mergable based on their set operation type 
and quantifier.

Review Comment:
   Typo in comment: 'mergable' should be 'mergeable'.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/iterative/rule/MergeIntersect.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.relational.planner.iterative.rule;
+
+import org.apache.iotdb.db.queryengine.plan.relational.planner.iterative.Rule;
+import 
org.apache.iotdb.db.queryengine.plan.relational.planner.node.IntersectNode;
+import org.apache.iotdb.db.queryengine.plan.relational.planner.node.Patterns;
+import 
org.apache.iotdb.db.queryengine.plan.relational.planner.node.SetOperationNode;
+import org.apache.iotdb.db.queryengine.plan.relational.utils.matching.Captures;
+import org.apache.iotdb.db.queryengine.plan.relational.utils.matching.Pattern;
+
+import java.util.Optional;
+
+public class MergeIntersect implements Rule<IntersectNode> {
+
+  private final Pattern<IntersectNode> pattern = Patterns.intersect();
+
+  @Override
+  public Pattern<IntersectNode> getPattern() {
+    return pattern;
+  }
+
+  @Override
+  public Result apply(IntersectNode node, Captures captures, Context context) {
+
+    SetOperationMerge mergeOperation = new SetOperationMerge(node, context);
+    Optional<SetOperationNode> result = mergeOperation.merge();
+    if (result.isPresent()) {
+      SetOperationNode setOperationNode = result.get();
+      return Result.ofPlanNode(setOperationNode);
+    }
+    return Result.empty();

Review Comment:
   [nitpick] Consider using a more concise pattern consistent with `MergeUnion` 
(line 46): `return result.map(Result::ofPlanNode).orElseGet(Result::empty);` 
instead of the if-else block. This improves code consistency across similar 
rule implementations.
   ```suggestion
       return result.map(Result::ofPlanNode).orElseGet(Result::empty);
   ```



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/iterative/rule/SetOperationMerge.java:
##########
@@ -124,11 +178,26 @@ private void addOriginalMappings(
   }
 
   /**
-   * Check if node and child are mergeable based on their set operation type 
and quantifier.
+   * Check if node and child are mergable based on their set operation type 
and quantifier.
+   *
+   * <p>For parent and child of type UNION, merge is always possible and the 
assumed quantifier is
+   * ALL, because UnionNode always represents UNION ALL.
+   *
+   * <p>For parent and child of type INTERSECT, merge is always possible: 1. 
if parent and child are
+   * both INTERSECT ALL, the resulting set operation is INTERSECT ALL 2. 
otherwise, the resulting
+   * set operation is INTERSECT DISTINCT: 3. if the parent is DISTINCT, the 
result has unique
+   * values, regardless of whether child branches were DISTINCT or ALL, 4. if 
the child is DISTINCT,
+   * that branch is guaranteed to have unique values, so at most one element 
of the other branches
+   * will be retained -- this is equivalent to just doing DISTINCT on the 
parent.

Review Comment:
   Incorrect numbering in documentation comment. The list items are numbered as 
"1. 2. 3. 4." but item 3 describes a sub-condition rather than standing alone. 
Consider restructuring for clarity: either merge items 3 and 4 as sub-bullets 
under item 2, or renumber as "2a. if the parent is DISTINCT..." and "2b. if the 
child is DISTINCT...".
   ```suggestion
      * <p>For parent and child of type INTERSECT, merge is always possible:
      *   1. If parent and child are both INTERSECT ALL, the resulting set 
operation is INTERSECT ALL.
      *   2. Otherwise, the resulting set operation is INTERSECT DISTINCT:
      *      a. If the parent is DISTINCT, the result has unique values, 
regardless of whether child branches were DISTINCT or ALL.
      *      b. If the child is DISTINCT, that branch is guaranteed to have 
unique values, so at most one element of the other branches will be retained -- 
this is equivalent to just doing DISTINCT on the parent.
   ```



##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/EvaluateEmptyIntersectTest.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.relational.analyzer;
+
+import org.apache.iotdb.db.queryengine.plan.relational.planner.PlanTester;
+
+import org.junit.Test;
+
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanAssert.assertPlan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.limit;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.output;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.tableScan;
+
+public class EvaluateEmptyIntersectTest {
+
+  @Test
+  public void EvaluateEmptyIntersectTest() {

Review Comment:
   Test method name should follow Java naming conventions. Method names should 
start with a lowercase letter. Consider renaming `EvaluateEmptyIntersectTest()` 
to `evaluateEmptyIntersectTest()`.
   ```suggestion
     public void evaluateEmptyIntersectTest() {
   ```



##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/MergeIntersectTest.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.relational.analyzer;
+
+import org.apache.iotdb.db.queryengine.plan.relational.planner.PlanTester;
+
+import org.junit.Test;
+
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanAssert.assertPlan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.aggregation;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.filter;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.output;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.project;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.sort;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.tableScan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.union;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.window;
+
+/**
+ * For parent and child of type INTERSECT, merge is always possible: 1. if 
parent and child are both
+ * INTERSECT ALL, the resulting set operation is INTERSECT ALL 2. otherwise, 
the resulting set
+ * operation is INTERSECT DISTINCT: 3. if the parent is DISTINCT, the result 
has unique values,
+ * regardless of whether child branches were DISTINCT or ALL, 4. if the child 
is DISTINCT, that
+ * branch is guaranteed to have unique values, so at most one element of the 
other branches will be
+ * retained -- this is equivalent to just doing DISTINCT on the parent.
+ */
+public class MergeIntersectTest {
+
+  @Test
+  public void MergeTwoIntersectAll() {

Review Comment:
   Test method name should follow Java naming conventions. Method names should 
start with a lowercase letter. Consider renaming `MergeTwoIntersectAll()` to 
`mergeTwoIntersectAll()`.
   ```suggestion
     public void mergeTwoIntersectAll() {
   ```



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/iterative/rule/SetOperationMerge.java:
##########
@@ -124,11 +178,26 @@ private void addOriginalMappings(
   }
 
   /**
-   * Check if node and child are mergeable based on their set operation type 
and quantifier.
+   * Check if node and child are mergable based on their set operation type 
and quantifier.

Review Comment:
   Typo in comment: 'mergable' should be 'mergeable'.



##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/MergeIntersectTest.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.relational.analyzer;
+
+import org.apache.iotdb.db.queryengine.plan.relational.planner.PlanTester;
+
+import org.junit.Test;
+
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanAssert.assertPlan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.aggregation;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.filter;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.output;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.project;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.sort;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.tableScan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.union;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.window;
+
+/**
+ * For parent and child of type INTERSECT, merge is always possible: 1. if 
parent and child are both
+ * INTERSECT ALL, the resulting set operation is INTERSECT ALL 2. otherwise, 
the resulting set
+ * operation is INTERSECT DISTINCT: 3. if the parent is DISTINCT, the result 
has unique values,
+ * regardless of whether child branches were DISTINCT or ALL, 4. if the child 
is DISTINCT, that
+ * branch is guaranteed to have unique values, so at most one element of the 
other branches will be
+ * retained -- this is equivalent to just doing DISTINCT on the parent.
+ */
+public class MergeIntersectTest {
+
+  @Test
+  public void MergeTwoIntersectAll() {
+    PlanTester planTester = new PlanTester();
+
+    // the parent
+    assertPlan(
+        planTester.createPlan(
+            "select tag1 from t1 intersect all select tag1 from t2  intersect 
all select tag1 from t3 "),
+        output(
+            project(
+                filter(
+                    project(
+                        window(
+                            sort(
+                                union(
+                                    project(tableScan("testdb.t1")),
+                                    project(tableScan("testdb.t2")),
+                                    project(tableScan("testdb.t3"))))))))));
+  }
+
+  @Test
+  public void MergeTwoIntersectDistinct() {

Review Comment:
   Test method name should follow Java naming conventions. Method names should 
start with a lowercase letter. Consider renaming `MergeTwoIntersectDistinct()` 
to `mergeTwoIntersectDistinct()`.
   ```suggestion
     public void mergeTwoIntersectDistinct() {
   ```



##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/MergeIntersectTest.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.relational.analyzer;
+
+import org.apache.iotdb.db.queryengine.plan.relational.planner.PlanTester;
+
+import org.junit.Test;
+
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanAssert.assertPlan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.aggregation;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.filter;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.output;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.project;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.sort;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.tableScan;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.union;
+import static 
org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.window;
+
+/**
+ * For parent and child of type INTERSECT, merge is always possible: 1. if 
parent and child are both
+ * INTERSECT ALL, the resulting set operation is INTERSECT ALL 2. otherwise, 
the resulting set
+ * operation is INTERSECT DISTINCT: 3. if the parent is DISTINCT, the result 
has unique values,
+ * regardless of whether child branches were DISTINCT or ALL, 4. if the child 
is DISTINCT, that
+ * branch is guaranteed to have unique values, so at most one element of the 
other branches will be
+ * retained -- this is equivalent to just doing DISTINCT on the parent.
+ */
+public class MergeIntersectTest {
+
+  @Test
+  public void MergeTwoIntersectAll() {
+    PlanTester planTester = new PlanTester();
+
+    // the parent
+    assertPlan(
+        planTester.createPlan(
+            "select tag1 from t1 intersect all select tag1 from t2  intersect 
all select tag1 from t3 "),
+        output(
+            project(
+                filter(
+                    project(
+                        window(
+                            sort(
+                                union(
+                                    project(tableScan("testdb.t1")),
+                                    project(tableScan("testdb.t2")),
+                                    project(tableScan("testdb.t3"))))))))));
+  }
+
+  @Test
+  public void MergeTwoIntersectDistinct() {
+    PlanTester planTester = new PlanTester();
+
+    // the parent
+    assertPlan(
+        planTester.createPlan(
+            "select tag1 from t1 intersect distinct select tag1 from t2  
intersect distinct select tag1 from t3 "),
+        output(
+            project(
+                filter(
+                    aggregation(
+                        union(
+                            project(tableScan("testdb.t1")),
+                            project(tableScan("testdb.t2")),
+                            project(tableScan("testdb.t3"))))))));
+  }
+
+  @Test
+  public void MergeIntersectAllAndIntersectDistinct() {

Review Comment:
   Test method name should follow Java naming conventions. Method names should 
start with a lowercase letter. Consider renaming 
`MergeIntersectAllAndIntersectDistinct()` to 
`mergeIntersectAllAndIntersectDistinct()`.
   ```suggestion
     public void mergeIntersectAllAndIntersectDistinct() {
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to