JackieTien97 commented on code in PR #14755: URL: https://github.com/apache/iotdb/pull/14755#discussion_r1948339608
########## iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/transformation/dag/column/multi/AbstractGreatestLeastColumnTransformer.java: ########## @@ -0,0 +1,116 @@ +/* + * Licensed 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.transformation.dag.column.multi; + +import org.apache.iotdb.db.queryengine.transformation.dag.column.ColumnTransformer; + +import org.apache.tsfile.block.column.Column; +import org.apache.tsfile.block.column.ColumnBuilder; +import org.apache.tsfile.read.common.type.Type; +import org.apache.tsfile.read.common.type.TypeEnum; + +import java.util.List; + +public abstract class AbstractGreatestLeastColumnTransformer extends MultiColumnTransformer { + + protected AbstractGreatestLeastColumnTransformer( + Type returnType, List<ColumnTransformer> columnTransformerList) { + super(returnType, columnTransformerList); + } + + @Override + protected void doTransform( + List<Column> childrenColumns, ColumnBuilder builder, int positionCount) { + for (int i = 0; i < positionCount; i++) { + final int index = i; + if (childrenColumns.stream().allMatch(column -> column.isNull(index))) { + builder.appendNull(); + continue; + } Review Comment: no need to do it here? It seems that you've already done it in the following `transform` ########## iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/transformation/dag/column/multi/BinaryGreatestColumnTransformer.java: ########## @@ -0,0 +1,53 @@ +/* + * Licensed 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.transformation.dag.column.multi; + +import org.apache.iotdb.db.queryengine.transformation.dag.column.ColumnTransformer; + +import org.apache.tsfile.block.column.Column; +import org.apache.tsfile.block.column.ColumnBuilder; +import org.apache.tsfile.read.common.type.Type; +import org.apache.tsfile.utils.Binary; + +import java.util.List; + +public class BinaryGreatestColumnTransformer extends AbstractGreatestLeastColumnTransformer { + protected BinaryGreatestColumnTransformer( + Type returnType, List<ColumnTransformer> columnTransformerList) { + super(returnType, columnTransformerList); + } + + @Override + protected void transform(ColumnBuilder builder, List<Column> childrenColumns, int index) { + Column firstColumn = childrenColumns.get(0); + Binary maxValue = firstColumn.getBinary(index); Review Comment: what if first value is null? The following `value.compareTo(maxValue)` won't be right ########## iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/transformation/dag/column/multi/AbstractGreatestLeastColumnTransformer.java: ########## @@ -0,0 +1,116 @@ +/* + * Licensed 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.transformation.dag.column.multi; + +import org.apache.iotdb.db.queryengine.transformation.dag.column.ColumnTransformer; + +import org.apache.tsfile.block.column.Column; +import org.apache.tsfile.block.column.ColumnBuilder; +import org.apache.tsfile.read.common.type.Type; +import org.apache.tsfile.read.common.type.TypeEnum; + +import java.util.List; + +public abstract class AbstractGreatestLeastColumnTransformer extends MultiColumnTransformer { + + protected AbstractGreatestLeastColumnTransformer( + Type returnType, List<ColumnTransformer> columnTransformerList) { + super(returnType, columnTransformerList); + } + + @Override + protected void doTransform( + List<Column> childrenColumns, ColumnBuilder builder, int positionCount) { + for (int i = 0; i < positionCount; i++) { + final int index = i; + if (childrenColumns.stream().allMatch(column -> column.isNull(index))) { + builder.appendNull(); + continue; + } + + transform(builder, childrenColumns, index); + } + } + + @Override + protected void doTransform( + List<Column> childrenColumns, ColumnBuilder builder, int positionCount, boolean[] selection) { + for (int i = 0; i < positionCount; i++) { + final int index = i; + if (selection[index] || childrenColumns.stream().allMatch(column -> column.isNull(index))) { Review Comment: right? selection[index] is true, and then you appendNull? ########## iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/metadata/TableMetadataImpl.java: ########## @@ -884,6 +894,28 @@ public static boolean isTwoTypeComparable(List<? extends Type> argumentTypes) { || ((isNumericType(left) || isCharType(left)) && isUnknownType(right)); } + public static boolean areAllTypesSameAndComparable(List<? extends Type> argumentTypes) { + if (argumentTypes == null || argumentTypes.isEmpty()) { + return true; + } + Type firstType = argumentTypes.get(0); + if (!firstType.isComparable()) { + return false; + } + return argumentTypes.stream().allMatch(type -> type.equals(firstType)); + } + + public static boolean isAllTypeComparable(List<? extends Type> argumentTypes) { + Type firstType = argumentTypes.get(0); + boolean flag = true; + for (Type type : argumentTypes) { + if (!isTwoTypeComparable(Arrays.asList(firstType, type))) { + flag = false; + } + } + return flag; + } + Review Comment: ```suggestion ``` -- 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]
