zstan commented on code in PR #2047: URL: https://github.com/apache/ignite-3/pull/2047#discussion_r1203873142
########## modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/NativeTypeValues.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.ignite.internal.sql.engine.util; + +import java.math.BigDecimal; +import java.time.Duration; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.Period; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.ignite.internal.schema.NativeType; +import org.apache.ignite.internal.sql.engine.type.IgniteCustomType; +import org.apache.ignite.internal.sql.engine.type.UuidType; +import org.jetbrains.annotations.Nullable; + +/** + * Produces values of {@link NativeType}s. + */ +public final class NativeTypeValues { + + private NativeTypeValues() { + + } + + /** Returns a list of values of a {@link NativeType} that corresponds to the given {@link RelDataType}. */ + public static List<Object> values(RelDataType type, int count) { + List<Object> result = new ArrayList<>(count); + for (var i = 0; i < count; i++) { + result.add(value(type, i)); + } + return result; + } + + /** Returns a value of a {@link NativeType} that corresponds to the given {@link RelDataType}. */ + @Nullable + public static Object value(RelDataType type, int i) { + switch (type.getSqlTypeName()) { + case BOOLEAN: + return true; Review Comment: nitpicking : ```suggestion return i == 0 ? false : true; ``` ########## modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/NativeTypeValues.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.ignite.internal.sql.engine.util; + +import java.math.BigDecimal; +import java.time.Duration; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.Period; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.ignite.internal.schema.NativeType; +import org.apache.ignite.internal.sql.engine.type.IgniteCustomType; +import org.apache.ignite.internal.sql.engine.type.UuidType; +import org.jetbrains.annotations.Nullable; + +/** + * Produces values of {@link NativeType}s. + */ +public final class NativeTypeValues { + + private NativeTypeValues() { + + } + + /** Returns a list of values of a {@link NativeType} that corresponds to the given {@link RelDataType}. */ + public static List<Object> values(RelDataType type, int count) { + List<Object> result = new ArrayList<>(count); + for (var i = 0; i < count; i++) { + result.add(value(type, i)); + } + return result; + } + + /** Returns a value of a {@link NativeType} that corresponds to the given {@link RelDataType}. */ + @Nullable + public static Object value(RelDataType type, int i) { + switch (type.getSqlTypeName()) { + case BOOLEAN: + return true; + case TINYINT: + return (byte) i; + case SMALLINT: + return (short) i; + case INTEGER: + return i; + case BIGINT: + return (long) i; + case DECIMAL: + return new BigDecimal("0.02").add(BigDecimal.ONE); Review Comment: why not : new BigDecimal(i) ? ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteTypeCoercion.java: ########## @@ -113,6 +146,78 @@ public boolean binaryComparisonCoercion(SqlCallBinding binding) { } } + /** {@inheritDoc} */ + @Override + public boolean binaryArithmeticCoercion(SqlCallBinding binding) { + ContextStack ctxStack = contextStack.get(); + Context ctx = ctxStack.push(ContextType.UNSPECIFIED); + try { + return super.binaryArithmeticCoercion(binding); + } finally { + ctxStack.pop(ctx); + } + } + + /** {@inheritDoc} */ + @Override + public boolean builtinFunctionCoercion(SqlCallBinding binding, List<RelDataType> operandTypes, List<SqlTypeFamily> expectedFamilies) { Review Comment: seems this code have no tests ? isn`t it ? ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/type/IgniteTypeSystem.java: ########## @@ -148,4 +149,37 @@ public RelDataType deriveSumType(RelDataTypeFactory typeFactory, RelDataType arg return typeFactory.createTypeWithNullability(sumType, argumentType.isNullable()); } + + /** + * Checks that {@code toType} and {@code fromType} have compatible type families taking into account custom data types. + * + * @see SqlTypeUtil#canAssignFrom(RelDataType, RelDataType) + */ + public boolean typeFamiliesAreCompatible(RelDataTypeFactory typeFactory, RelDataType toType, RelDataType fromType) { + // Types T1 and T2 have compatible type families if their types are assignable. + + // Same types are always compatible. + if (SqlTypeUtil.equalSansNullability(typeFactory, toType, fromType)) { + return true; + } + + // NULL is compatible with all types. + if (fromType.getSqlTypeName() == SqlTypeName.NULL || toType.getSqlTypeName() == SqlTypeName.NULL) { + return true; + } else if (fromType instanceof IgniteCustomType && toType instanceof IgniteCustomType) { + IgniteCustomType fromCustom = (IgniteCustomType) fromType; + IgniteCustomType toCustom = (IgniteCustomType) toType; + + // IgniteCustomType: different custom data types are not compatible. + return Objects.equals(fromCustom.getCustomTypeName(), toCustom.getCustomTypeName()); + } else if (fromType instanceof IgniteCustomType || toType instanceof IgniteCustomType) { + // Custom data types are not compatible with other types. + return false; + } else { + boolean fromTo = SqlTypeUtil.canAssignFrom(toType, fromType); Review Comment: I change this function into impl near and all tests from IgniteTypeSystemTest are passed , we need to change implementation or append new tests. `if (fromType instanceof IgniteCustomType && toType instanceof IgniteCustomType) { IgniteCustomType fromCustom = (IgniteCustomType) fromType; IgniteCustomType toCustom = (IgniteCustomType) toType; // IgniteCustomType: different custom data types are not compatible. return Objects.equals(fromCustom.getCustomTypeName(), toCustom.getCustomTypeName()); } else return SqlTypeUtil.inSameFamilyOrNull(toType, fromType);` ########## modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/FunctionsTest.java: ########## @@ -0,0 +1,99 @@ +/* + * 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.ignite.internal.sql.engine.planner; + +import java.math.BigDecimal; +import java.util.UUID; +import java.util.stream.Stream; +import org.apache.ignite.internal.sql.engine.util.StatementChecker; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.DynamicTest; +import org.junit.jupiter.api.TestFactory; + +/** + * Function call validation. + */ +public class FunctionsTest extends AbstractPlannerTest { Review Comment: why only SUBSTR present here ? ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteTypeCoercion.java: ########## @@ -113,6 +146,78 @@ public boolean binaryComparisonCoercion(SqlCallBinding binding) { } } + /** {@inheritDoc} */ + @Override + public boolean binaryArithmeticCoercion(SqlCallBinding binding) { + ContextStack ctxStack = contextStack.get(); + Context ctx = ctxStack.push(ContextType.UNSPECIFIED); + try { + return super.binaryArithmeticCoercion(binding); + } finally { + ctxStack.pop(ctx); + } + } + + /** {@inheritDoc} */ + @Override + public boolean builtinFunctionCoercion(SqlCallBinding binding, List<RelDataType> operandTypes, List<SqlTypeFamily> expectedFamilies) { + ContextStack ctxStack = contextStack.get(); + Context ctx = ctxStack.push(ContextType.UNSPECIFIED); + try { + return super.builtinFunctionCoercion(binding, operandTypes, expectedFamilies); + } finally { + ctxStack.pop(ctx); + } + } + + /** {@inheritDoc} */ + @Override + public boolean userDefinedFunctionCoercion(SqlValidatorScope scope, SqlCall call, SqlFunction function) { Review Comment: seems this code have no tests ? isn`t it ? -- 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]
