IGNITE-27612 Allow numeric primitive type conversion when reading values from and writing values to Tuple#7630
IGNITE-27612 Allow numeric primitive type conversion when reading values from and writing values to Tuple#7630xtern wants to merge 4 commits intoapache:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements implicit numeric type conversions for Tuple operations, allowing both reading from and writing to tuples with automatic type casting. The feature supports widening conversions (e.g., byte→short→int→long for integers, float→double for floating-point) that are always safe, as well as narrowing conversions (e.g., long→int→short→byte, double→float) with overflow/precision checks that throw ArithmeticException when the value doesn't fit the target type.
Changes:
- Introduced
TupleTypeCastUtilsutility class in the core module with methods for safe numeric type conversions - Updated all Tuple implementations (AbstractRowTupleAdapter, AsyncResultSetImpl.SqlRowImpl, MutableTupleBinaryTupleAdapter, TupleImpl) to use type casting when reading primitive values
- Modified write path (Column.validate, RowAssembler.appendValue, ClientBinaryTupleUtils.appendValue) to accept compatible numeric types with validation
- Added comprehensive integration and unit tests covering all conversion scenarios and edge cases (overflow, infinity, NaN)
- Updated Tuple API documentation to describe the new implicit conversion behavior
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/core/src/main/java/org/apache/ignite/internal/util/TupleTypeCastUtils.java | New utility class implementing core type casting logic with range/precision checks |
| modules/table/src/main/java/org/apache/ignite/internal/table/AbstractRowTupleAdapter.java | Modified to use TupleTypeCastUtils for reading numeric values with type casting |
| modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/AsyncResultSetImpl.java | Updated SqlRowImpl to support type casting when reading values; fixed valueOrDefault implementation; added equals/hashCode |
| modules/client-common/src/main/java/org/apache/ignite/internal/client/table/MutableTupleBinaryTupleAdapter.java | Refactored to use TupleTypeCastUtils for reading numeric values with casting |
| modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientBinaryTupleUtils.java | Modified to use TupleTypeCastUtils when writing values, catches ArithmeticException |
| modules/schema/src/main/java/org/apache/ignite/internal/schema/Column.java | Added isCastAllowed check before rejecting type mismatches during validation |
| modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java | Changed direct casts to Number conversion methods (byteValue(), shortValue(), etc.) |
| modules/api/src/main/java/org/apache/ignite/table/TupleImpl.java | Duplicated casting logic from TupleTypeCastUtils (necessary due to module boundaries) |
| modules/api/src/main/java/org/apache/ignite/table/Tuple.java | Added documentation describing implicit numeric type conversion behavior |
| modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItRecordBinaryViewApiTest.java | Added comprehensive integration tests for type casting; modified existing test data |
| modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItKeyValueBinaryViewApiTest.java | Added comprehensive integration tests for type casting in key-value views; modified existing test data |
| modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/api/SqlRowTest.java | Refactored to extend AbstractImmutableTupleTest, inheriting comprehensive type casting tests |
| modules/client/src/test/java/org/apache/ignite/internal/client/sql/ClientSqlRowTest.java | Refactored to extend AbstractImmutableTupleTest, inheriting comprehensive type casting tests |
| modules/api/src/testFixtures/java/org/apache/ignite/table/AbstractImmutableTupleTest.java | Added extensive parameterized tests for all numeric type conversions including edge cases |
| modules/schema/src/testFixtures/java/org/apache/ignite/internal/schema/SchemaTestUtils.java | Removed PRIMITIVE_ACCESSORS map (moved to individual test classes) |
| modules/client-handler/src/test/java/org/apache/ignite/client/handler/requests/table/ClientHandlerTupleTests.java | Updated tests to accommodate new boolean column in schema; recreated PRIMITIVE_ACCESSORS locally |
| modules/sql-engine/build.gradle | Added test dependency on ignite-api testFixtures for AbstractImmutableTupleTest |
| modules/api/src/test/java/org/apache/ignite/table/TupleImplTest.java | Disabled allTypesUnsupportedConversion test (tracked by IGNITE-27577) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ble/src/integrationTest/java/org/apache/ignite/internal/table/ItRecordBinaryViewApiTest.java
Show resolved
Hide resolved
...e/src/integrationTest/java/org/apache/ignite/internal/table/ItKeyValueBinaryViewApiTest.java
Show resolved
Hide resolved
...ble/src/integrationTest/java/org/apache/ignite/internal/table/ItRecordBinaryViewApiTest.java
Show resolved
Hide resolved
...e/src/integrationTest/java/org/apache/ignite/internal/table/ItKeyValueBinaryViewApiTest.java
Show resolved
Hide resolved
| } | ||
|
|
||
| /** Casts a {@link Number} to {@code byte}. */ | ||
| private static byte castToByte(Object number) { |
There was a problem hiding this comment.
Looks like similar methods are implemented in TupleTypeCaseUtils.
Let's move them to CastUtils of core module and reuse.
There was a problem hiding this comment.
Also, we have IgniteMath byte convertToByteExact(long x) method.
Maybe, we can reuse it.
However, one may be confised with overloaded method convertToByteExact(Number x), which has different semantic.
There was a problem hiding this comment.
TupleImpl is part of the public API module and can't depend on the core module.
On the other hand, introducing a new helper class into the API module is the wrong approach, thus we will maintain the duplication, but this is covered by tests (see AbstractImmutableTupleTest, AbstractMutableTupleTest) they cover all tuple implementations.
| @Override | ||
| @SuppressWarnings("JUnitMalformedDeclaration") | ||
| public void allTypesUnsupportedConversion(ColumnType from, ColumnType to) { | ||
| super.allTypesUnsupportedConversion(from, to); |
There was a problem hiding this comment.
Maybe we can check only exception class, rather than disable the test?
https://issues.apache.org/jira/browse/IGNITE-27612
This is a pull request to main branch previously reviewed PRs: #7400 and #7439