Copilot commented on code in PR #1959: URL: https://github.com/apache/auron/pull/1959#discussion_r2735005196
########## auron-flink-extension/auron-flink-runtime/src/test/java/org/apache/auron/flink/arrow/FlinkArrowUtilsTest.java: ########## @@ -0,0 +1,202 @@ +/* + * 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.auron.flink.arrow; + +import static org.junit.jupiter.api.Assertions.*; + +import org.apache.arrow.vector.types.DateUnit; +import org.apache.arrow.vector.types.FloatingPointPrecision; +import org.apache.arrow.vector.types.TimeUnit; +import org.apache.arrow.vector.types.pojo.ArrowType; +import org.apache.arrow.vector.types.pojo.Field; +import org.apache.arrow.vector.types.pojo.Schema; +import org.apache.flink.api.common.typeutils.base.StringSerializer; +import org.apache.flink.table.types.logical.ArrayType; +import org.apache.flink.table.types.logical.BigIntType; +import org.apache.flink.table.types.logical.BinaryType; +import org.apache.flink.table.types.logical.BooleanType; +import org.apache.flink.table.types.logical.CharType; +import org.apache.flink.table.types.logical.DateType; +import org.apache.flink.table.types.logical.DecimalType; +import org.apache.flink.table.types.logical.DoubleType; +import org.apache.flink.table.types.logical.FloatType; +import org.apache.flink.table.types.logical.IntType; +import org.apache.flink.table.types.logical.LocalZonedTimestampType; +import org.apache.flink.table.types.logical.LogicalType; +import org.apache.flink.table.types.logical.MapType; +import org.apache.flink.table.types.logical.RawType; +import org.apache.flink.table.types.logical.RowType; +import org.apache.flink.table.types.logical.SmallIntType; +import org.apache.flink.table.types.logical.TimeType; +import org.apache.flink.table.types.logical.TimestampType; +import org.apache.flink.table.types.logical.TinyIntType; +import org.apache.flink.table.types.logical.VarBinaryType; +import org.apache.flink.table.types.logical.VarCharType; +import org.junit.jupiter.api.Test; + +/** Unit tests for FlinkArrowUtils. */ +public class FlinkArrowUtilsTest { + + @Test + public void testBasicTypeConversion() { + // Boolean + assertEquals(ArrowType.Bool.INSTANCE, FlinkArrowUtils.toArrowType(new BooleanType())); + + // Integer types + assertEquals(new ArrowType.Int(8, true), FlinkArrowUtils.toArrowType(new TinyIntType())); + assertEquals(new ArrowType.Int(16, true), FlinkArrowUtils.toArrowType(new SmallIntType())); + assertEquals(new ArrowType.Int(32, true), FlinkArrowUtils.toArrowType(new IntType())); + assertEquals(new ArrowType.Int(64, true), FlinkArrowUtils.toArrowType(new BigIntType())); + + // Floating point types + assertEquals( + new ArrowType.FloatingPoint(FloatingPointPrecision.SINGLE), + FlinkArrowUtils.toArrowType(new FloatType())); + assertEquals( + new ArrowType.FloatingPoint(FloatingPointPrecision.DOUBLE), + FlinkArrowUtils.toArrowType(new DoubleType())); + + // String and binary types + assertEquals(ArrowType.Utf8.INSTANCE, FlinkArrowUtils.toArrowType(new VarCharType(100))); + assertEquals(ArrowType.Utf8.INSTANCE, FlinkArrowUtils.toArrowType(new CharType(10))); + assertEquals(ArrowType.Binary.INSTANCE, FlinkArrowUtils.toArrowType(new VarBinaryType(100))); + assertEquals(ArrowType.Binary.INSTANCE, FlinkArrowUtils.toArrowType(new BinaryType(10))); + + // Decimal type + DecimalType decimalType = new DecimalType(10, 2); + ArrowType arrowDecimal = FlinkArrowUtils.toArrowType(decimalType); + assertTrue(arrowDecimal instanceof ArrowType.Decimal); + assertEquals(10, ((ArrowType.Decimal) arrowDecimal).getPrecision()); + assertEquals(2, ((ArrowType.Decimal) arrowDecimal).getScale()); Review Comment: The test for DecimalType should also verify the bitWidth of the returned ArrowType.Decimal. According to the implementation comment in FlinkArrowUtils.java (lines 110-112), the bitWidth is explicitly set to 128 to match Arrow Java's actual storage. Consider adding an assertion like: assertEquals(128, ((ArrowType.Decimal) arrowDecimal).getBitWidth()); ########## auron-flink-extension/auron-flink-runtime/src/test/java/org/apache/auron/flink/arrow/FlinkArrowUtilsTest.java: ########## @@ -0,0 +1,202 @@ +/* + * 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.auron.flink.arrow; + +import static org.junit.jupiter.api.Assertions.*; + +import org.apache.arrow.vector.types.DateUnit; +import org.apache.arrow.vector.types.FloatingPointPrecision; +import org.apache.arrow.vector.types.TimeUnit; +import org.apache.arrow.vector.types.pojo.ArrowType; +import org.apache.arrow.vector.types.pojo.Field; +import org.apache.arrow.vector.types.pojo.Schema; +import org.apache.flink.api.common.typeutils.base.StringSerializer; +import org.apache.flink.table.types.logical.ArrayType; +import org.apache.flink.table.types.logical.BigIntType; +import org.apache.flink.table.types.logical.BinaryType; +import org.apache.flink.table.types.logical.BooleanType; +import org.apache.flink.table.types.logical.CharType; +import org.apache.flink.table.types.logical.DateType; +import org.apache.flink.table.types.logical.DecimalType; +import org.apache.flink.table.types.logical.DoubleType; +import org.apache.flink.table.types.logical.FloatType; +import org.apache.flink.table.types.logical.IntType; +import org.apache.flink.table.types.logical.LocalZonedTimestampType; +import org.apache.flink.table.types.logical.LogicalType; +import org.apache.flink.table.types.logical.MapType; +import org.apache.flink.table.types.logical.RawType; +import org.apache.flink.table.types.logical.RowType; +import org.apache.flink.table.types.logical.SmallIntType; +import org.apache.flink.table.types.logical.TimeType; +import org.apache.flink.table.types.logical.TimestampType; +import org.apache.flink.table.types.logical.TinyIntType; +import org.apache.flink.table.types.logical.VarBinaryType; +import org.apache.flink.table.types.logical.VarCharType; +import org.junit.jupiter.api.Test; + +/** Unit tests for FlinkArrowUtils. */ +public class FlinkArrowUtilsTest { + + @Test + public void testBasicTypeConversion() { + // Boolean + assertEquals(ArrowType.Bool.INSTANCE, FlinkArrowUtils.toArrowType(new BooleanType())); + + // Integer types + assertEquals(new ArrowType.Int(8, true), FlinkArrowUtils.toArrowType(new TinyIntType())); + assertEquals(new ArrowType.Int(16, true), FlinkArrowUtils.toArrowType(new SmallIntType())); + assertEquals(new ArrowType.Int(32, true), FlinkArrowUtils.toArrowType(new IntType())); + assertEquals(new ArrowType.Int(64, true), FlinkArrowUtils.toArrowType(new BigIntType())); + + // Floating point types + assertEquals( + new ArrowType.FloatingPoint(FloatingPointPrecision.SINGLE), + FlinkArrowUtils.toArrowType(new FloatType())); + assertEquals( + new ArrowType.FloatingPoint(FloatingPointPrecision.DOUBLE), + FlinkArrowUtils.toArrowType(new DoubleType())); + + // String and binary types + assertEquals(ArrowType.Utf8.INSTANCE, FlinkArrowUtils.toArrowType(new VarCharType(100))); + assertEquals(ArrowType.Utf8.INSTANCE, FlinkArrowUtils.toArrowType(new CharType(10))); + assertEquals(ArrowType.Binary.INSTANCE, FlinkArrowUtils.toArrowType(new VarBinaryType(100))); + assertEquals(ArrowType.Binary.INSTANCE, FlinkArrowUtils.toArrowType(new BinaryType(10))); + + // Decimal type + DecimalType decimalType = new DecimalType(10, 2); + ArrowType arrowDecimal = FlinkArrowUtils.toArrowType(decimalType); + assertTrue(arrowDecimal instanceof ArrowType.Decimal); + assertEquals(10, ((ArrowType.Decimal) arrowDecimal).getPrecision()); + assertEquals(2, ((ArrowType.Decimal) arrowDecimal).getScale()); + + // Date and timestamp types + assertEquals(new ArrowType.Date(DateUnit.DAY), FlinkArrowUtils.toArrowType(new DateType())); + assertEquals( + new ArrowType.Timestamp(TimeUnit.MICROSECOND, null), FlinkArrowUtils.toArrowType(new TimestampType(3))); + } Review Comment: The implementation handles NullType (line 88-89 in FlinkArrowUtils.java), but there is no test coverage for this type. Consider adding a test case: assertEquals(ArrowType.Null.INSTANCE, FlinkArrowUtils.toArrowType(new NullType())); -- 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]
