korlov42 commented on code in PR #5145:
URL: https://github.com/apache/ignite-3/pull/5145#discussion_r1940798892
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java:
##########
@@ -715,16 +717,22 @@ private static TypeSpec convertToTypeSpec(RelDataType
type) {
* <br>
* 2) If one or more of the rightmost M-L characters of V are not
space(s), then an
* exception condition is raised: data exception — string data, right
truncation.
+ * <br>
+ * 3) If the declared type of T is binary string and the length in octets
of V is greater than the
Review Comment:
you need to restructure javadoc: I believe this item doesn't relate to
`fixed-length character string`.
Besides, may you point at particular section from standard? Can't find this
paragraph in Store assignment section
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java:
##########
@@ -735,6 +743,15 @@ public static <RowT> RowT
validateCharactersOverflowAndTrimIfPossible(
RelDataType colType = rowType.getFieldList().get(i).getType();
Object data = rowHandler.get(i, row);
+ if (BINARY_TYPES.contains(colType.getSqlTypeName()) && data !=
null) {
+ assert data instanceof ByteString;
+ assert colType.getPrecision() !=
RelDataType.PRECISION_NOT_SPECIFIED;
+
+ if (((ByteString) data).length() > colType.getPrecision()) {
+ throw new SqlException(STMT_VALIDATION_ERR, format("Value
too long for type binary({})", colType.getPrecision()));
Review Comment:
agree with @lowka , these cases should share patter for error message
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDataTypesTest.java:
##########
@@ -629,18 +630,17 @@ private static Stream<Arguments>
decimalOverflowsValidation() {
}
@Test
- @SuppressWarnings("ThrowableNotThrown")
public void testCharTypesWithTrailingSpacesAreTrimmed() {
- sql("create table limitedChar (pk int primary key, f1 VARCHAR(3))");
+ sql("create table limitedChar (pk int primary key, fb1 VARBINARY(2),
f1 VARCHAR(3), fb2 VARBINARY(1))");
try {
- sql("insert into limitedChar values (1, 'a b ')");
+ sql("insert into limitedChar values (1, x'01', 'a b ',
x'01')");
assertQuery("select length(f1) from limitedChar")
.returns(3)
.check();
- sql("insert into limitedChar values (2, 'a ' || ?)", "b ");
+ sql("insert into limitedChar values (2, x'01', 'a ' || ?, x'01')",
"b ");
Review Comment:
unrelated changes
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteTypeCoercion.java:
##########
@@ -284,7 +284,11 @@ public boolean querySourceCoercion(
@Override
protected boolean needToCast(SqlValidatorScope scope, SqlNode node,
RelDataType toType) {
RelDataType fromType = validator.deriveType(scope, node);
- if (SqlTypeUtil.isInterval(toType)) {
+
+ // No need to cast between binary types.
+ if (SqlTypeUtil.isBinary(toType) && SqlTypeUtil.isBinary(fromType)) {
+ return false;
+ } else if (SqlTypeUtil.isInterval(toType)) {
Review Comment:
any changes in type coercion must be covered with TypeCoercion tests
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java:
##########
@@ -715,16 +717,22 @@ private static TypeSpec convertToTypeSpec(RelDataType
type) {
* <br>
* 2) If one or more of the rightmost M-L characters of V are not
space(s), then an
* exception condition is raised: data exception — string data, right
truncation.
+ * <br>
+ * 3) If the declared type of T is binary string and the length in octets
of V is greater than the
+ * maximum length in octets L of T, then the value of T is set to the
first L octets of V, the
+ * length in octets of T becomes L, and a completion condition is
raised: warning — string
+ * data, right truncation.
*/
- public static <RowT> RowT validateCharactersOverflowAndTrimIfPossible(
+ public static <RowT> RowT validateStringTypesOverflowAndTrimIfPossible(
Review Comment:
let's add unit test to `TypeUtilsTest` to cover BINARY_TYPES-related branches
--
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]
