Copilot commented on code in PR #17169:
URL: https://github.com/apache/iotdb/pull/17169#discussion_r2928308839
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/InformationSchema.java:
##########
@@ -388,10 +414,31 @@ public class InformationSchema {
schemaTables.put(SERVICES, servicesTable);
}
+ static {
+ columnsThatSupportPushDownPredicate.put(
+ TABLE_DISK_USAGE,
+ ImmutableSet.of(
+ ColumnHeaderConstant.DATABASE,
+ ColumnHeaderConstant.TABLE_NAME_TABLE_MODEL,
+ ColumnHeaderConstant.DATA_NODE_ID_TABLE_MODEL,
+ ColumnHeaderConstant.REGION_ID_TABLE_MODEL,
+ ColumnHeaderConstant.TIME_PARTITION_TABLE_MODEL));
Review Comment:
columnsThatSupportPushDownPredicate for information_schema.table_disk_usage
uses ColumnHeaderConstant.DATABASE ("Database"), but the table schema defines
this column as lowercase "database" (via
`ColumnHeaderConstant.DATABASE.toLowerCase(...)`). This case mismatch prevents
predicate pushdown on the database column. Use the same lowercase column name
here (or a dedicated table-model constant) so the pushdown checker matches
SymbolReference names correctly.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/process/CollectNode.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.planner.plan.node.process;
+
+import org.apache.iotdb.db.queryengine.plan.planner.plan.node.PlanNode;
+import org.apache.iotdb.db.queryengine.plan.planner.plan.node.PlanNodeId;
+import org.apache.iotdb.db.queryengine.plan.planner.plan.node.PlanNodeType;
+import org.apache.iotdb.db.queryengine.plan.planner.plan.node.PlanVisitor;
+
+import com.google.common.base.Objects;
+import org.apache.tsfile.utils.ReadWriteIOUtils;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+public class CollectNode extends MultiChildProcessNode {
+
+ private final List<String> outputColumnNames;
+
+ public CollectNode(PlanNodeId id, List<String> outputColumnNames) {
+ super(id);
+ this.outputColumnNames = outputColumnNames;
+ }
+
+ public CollectNode(PlanNodeId id, List<PlanNode> children, List<String>
outputColumnNames) {
+ super(id, children);
+ this.outputColumnNames = outputColumnNames;
+ }
+
+ @Override
+ public <R, C> R accept(PlanVisitor<R, C> visitor, C context) {
+ return visitor.visitCollect(this, context);
+ }
+
+ @Override
+ public PlanNode clone() {
+ return new CollectNode(id, outputColumnNames);
+ }
+
+ @Override
+ public PlanNodeType getType() {
+ return PlanNodeType.TREE_COLLECT;
+ }
+
+ @Override
+ public List<String> getOutputColumnNames() {
+ return outputColumnNames;
+ }
+
+ @Override
+ public PlanNode replaceChildren(List<PlanNode> newChildren) {
+ checkArgument(children.size() == newChildren.size(), "wrong number of new
children");
+ return new CollectNode(id, newChildren, outputColumnNames);
+ }
+
+ @Override
+ protected void serializeAttributes(ByteBuffer byteBuffer) {
+ PlanNodeType.TREE_COLLECT.serialize(byteBuffer);
+ ReadWriteIOUtils.write(outputColumnNames.size(), byteBuffer);
+ outputColumnNames.forEach(column -> ReadWriteIOUtils.write(column,
byteBuffer));
+ }
+
+ @Override
+ protected void serializeAttributes(DataOutputStream stream) throws
IOException {
+ PlanNodeType.TREE_COLLECT.serialize(stream);
+ ReadWriteIOUtils.write(outputColumnNames.size(), stream);
+ for (String outputColumnName : outputColumnNames) {
+ ReadWriteIOUtils.write(outputColumnName, stream);
+ }
+ }
+
+ public static CollectNode deserialize(ByteBuffer byteBuffer) {
+ int size = ReadWriteIOUtils.readInt(byteBuffer);
+ List<String> outputColumnNames = new ArrayList<>(size);
+ while (size-- > 0) {
+ outputColumnNames.add(ReadWriteIOUtils.readString(byteBuffer));
+ }
+ PlanNodeId planNodeId = PlanNodeId.deserialize(byteBuffer);
+ return new CollectNode(planNodeId, outputColumnNames);
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ if (!super.equals(o)) return false;
+ return false;
+ }
Review Comment:
CollectNode.equals() currently returns false for all non-identical instances
(it never compares any fields and ends with `return false`). This breaks the
equals/hashCode contract and can cause incorrect plan comparisons/caching and
brittle tests. Implement equals() to compare relevant state (at least
superclass + outputColumnNames, and typically children via super.equals).
--
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]
