This is an automated email from the ASF dual-hosted git repository.
jooger pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 6f71d3a314 IGNITE-22451: Sql. An insert may cause specific later
updates to fail. Affects only char/varchar (#3912)
6f71d3a314 is described below
commit 6f71d3a314b8dd1df0496dfff4365f4d02c8824d
Author: Max Zhuravkov <[email protected]>
AuthorDate: Fri Jun 14 15:26:05 2024 +0400
IGNITE-22451: Sql. An insert may cause specific later updates to fail.
Affects only char/varchar (#3912)
---
.../sql/types/char/test_implicit_cast.test | 3 -
.../ignite/internal/sql/engine/util/TypeUtils.java | 45 ++++---
.../internal/sql/engine/util/TypeUtilsTest.java | 142 +++++++++++++++++++++
3 files changed, 164 insertions(+), 26 deletions(-)
diff --git
a/modules/sql-engine/src/integrationTest/sql/types/char/test_implicit_cast.test
b/modules/sql-engine/src/integrationTest/sql/types/char/test_implicit_cast.test
index 024def72e9..8a155d0ad5 100644
---
a/modules/sql-engine/src/integrationTest/sql/types/char/test_implicit_cast.test
+++
b/modules/sql-engine/src/integrationTest/sql/types/char/test_implicit_cast.test
@@ -129,8 +129,5 @@ INSERT INTO tmp (c5) VALUES('a'::CHAR(5));
statement ok
INSERT INTO tmp (c5) VALUES('a'::CHAR(3));
-# https://issues.apache.org/jira/browse/IGNITE-22451
-# Error: Row has not been fully built. Index: 2, fields: 3
-skipif ignite3
statement ok
UPDATE tmp SET c5=c5::CHAR(6)
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java
index 0877a04ae2..5d6e713d59 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java
@@ -698,40 +698,39 @@ public class TypeUtils {
RelDataType colType = rowType.getFieldList().get(i).getType();
Object data = rowHandler.get(i, row);
- if (CHAR_TYPES.contains(colType.getSqlTypeName())) {
- if (data == null) {
- continue;
+ // Skip null values and non-character types.
+ if (!CHAR_TYPES.contains(colType.getSqlTypeName()) || data ==
null) {
+ if (rowBldr != null) {
+ rowBldr.addField(data);
}
+ continue;
+ }
+ // Otherwise validate and trim if needed.
- assert data instanceof String;
+ assert data instanceof String;
- String str = (String) data;
+ String str = (String) data;
- int colPrecision = colType.getPrecision();
+ int colPrecision = colType.getPrecision();
- assert colPrecision != RelDataType.PRECISION_NOT_SPECIFIED;
+ assert colPrecision != RelDataType.PRECISION_NOT_SPECIFIED;
- if (str.length() > colPrecision) {
- for (int pos = str.length(); pos > colPrecision; --pos) {
- if (str.charAt(pos - 1) != ' ') {
- throw new SqlException(STMT_VALIDATION_ERR, "Value
too long for type: " + colType);
- }
+ if (str.length() > colPrecision) {
+ for (int pos = str.length(); pos > colPrecision; --pos) {
+ if (str.charAt(pos - 1) != ' ') {
+ throw new SqlException(STMT_VALIDATION_ERR, "Value too
long for type: " + colType);
}
+ }
- str = str.substring(0, colPrecision);
+ str = str.substring(0, colPrecision);
- if (rowBldr == null) {
- rowBldr = buildPartialRow(rowHandler, schema, i, row);
- }
+ if (rowBldr == null) {
+ rowBldr = buildPartialRow(rowHandler, schema, i, row);
}
+ }
- if (rowBldr != null) {
- rowBldr.addField(str);
- }
- } else {
- if (rowBldr != null) {
- rowBldr.addField(data);
- }
+ if (rowBldr != null) {
+ rowBldr.addField(str);
}
}
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/TypeUtilsTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/TypeUtilsTest.java
index 9c12bd103a..4c2580dcd8 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/TypeUtilsTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/TypeUtilsTest.java
@@ -18,6 +18,8 @@
package org.apache.ignite.internal.sql.engine.util;
import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
+import static
org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -30,10 +32,12 @@ import java.time.LocalTime;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
+import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeFactory.Builder;
+import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.sql.SqlIntervalQualifier;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.sql.type.SqlTypeName;
@@ -42,14 +46,17 @@ import
org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
import org.apache.ignite.internal.sql.engine.exec.row.RowType;
import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.framework.ArrayRowHandler;
import org.apache.ignite.internal.sql.engine.type.IgniteCustomType;
import org.apache.ignite.internal.sql.engine.type.IgniteCustomTypeSpec;
import org.apache.ignite.internal.sql.engine.type.IgniteTypeFactory;
import org.apache.ignite.internal.sql.engine.type.UuidType;
import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
import org.apache.ignite.internal.type.NativeTypes;
+import org.apache.ignite.lang.ErrorGroups.Sql;
import org.apache.ignite.sql.ColumnType;
import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestFactory;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
@@ -76,6 +83,141 @@ public class TypeUtilsTest extends BaseIgniteAbstractTest {
private static final BaseTypeSpec BYTES =
RowSchemaTypes.nativeType(NativeTypes.BYTES);
private static final BaseTypeSpec UUID =
RowSchemaTypes.nativeType(NativeTypes.UUID);
+ @Test
+ public void testValidateCharactersOverflowAndTrimIfPossible() {
+ IgniteTypeFactory typeFactory = Commons.typeFactory();
+
+ {
+ RelDataType rowType = typeFactory.builder()
+ .add("c1", typeFactory.createSqlType(SqlTypeName.VARCHAR,
3))
+ .add("c2", typeFactory.createSqlType(SqlTypeName.VARCHAR,
6))
+ .build();
+
+ Object[] input = {"123 ", "12345 "};
+ Object[] expected = {"123", "12345 "};
+
+ expectOutputRow(rowType, input, expected);
+ }
+
+ {
+ RelDataType rowType = typeFactory.builder()
+ .add("c1", typeFactory.createSqlType(SqlTypeName.VARCHAR,
4))
+ .build();
+
+ Object[] input = {" 12 "};
+ Object[] expected = {" 12 "};
+
+ expectOutputRow(rowType, input, expected);
+ }
+
+ {
+ RelDataType rowType = typeFactory.builder()
+ .add("c1", typeFactory.createSqlType(SqlTypeName.VARCHAR,
6))
+ .add("c2", typeFactory.createSqlType(SqlTypeName.VARCHAR,
5))
+ .build();
+
+ Object[] input = {null, "12345 "};
+ Object[] expected = {null, "12345"};
+
+ expectOutputRow(rowType, input, expected);
+ }
+
+ {
+ RelDataType rowType = typeFactory.builder()
+ .add("c1", typeFactory.createSqlType(SqlTypeName.VARCHAR,
6))
+ .add("c2", typeFactory.createSqlType(SqlTypeName.VARCHAR,
5))
+ .build();
+
+ Object[] input = {"12345 ", null};
+ Object[] expected = {"12345 ", null};
+
+ expectOutputRow(rowType, input, expected);
+ }
+
+ {
+ RelDataType rowType = typeFactory.builder()
+ .add("c1", typeFactory.createSqlType(SqlTypeName.VARCHAR,
6))
+ .add("c2", typeFactory.createSqlType(SqlTypeName.INTEGER))
+ .build();
+
+ Object[] input = {"12345 ", null};
+ Object[] expected = {"12345 ", null};
+
+ expectOutputRow(rowType, input, expected);
+ }
+
+ {
+ RelDataType rowType = typeFactory.builder()
+ .add("c1", typeFactory.createSqlType(SqlTypeName.VARCHAR,
6))
+ .add("c2", typeFactory.createSqlType(SqlTypeName.VARCHAR,
7))
+ .add("c3", typeFactory.createSqlType(SqlTypeName.VARCHAR,
3))
+ .build();
+
+ Object[] input = {"12345 ", null, "123 "};
+ Object[] expected = {"12345 ", null, "123"};
+
+ expectOutputRow(rowType, input, expected);
+ }
+
+ {
+ RelDataType rowType = typeFactory.builder()
+ .add("c1", typeFactory.createSqlType(SqlTypeName.VARCHAR,
6))
+ .add("c2", typeFactory.createSqlType(SqlTypeName.INTEGER))
+ .add("c3", typeFactory.createSqlType(SqlTypeName.VARCHAR,
3))
+ .build();
+
+ Object[] input = {"12345 ", null, "123 "};
+ Object[] expected = {"12345 ", null, "123"};
+
+ expectOutputRow(rowType, input, expected);
+ }
+
+
+ {
+ RelDataType rowType = typeFactory.builder()
+ .add("c1", typeFactory.createSqlType(SqlTypeName.INTEGER))
+ .build();
+
+ Object[] input = {2};
+ Object[] expected = {2};
+
+ expectOutputRow(rowType, input, expected);
+ }
+
+ {
+ RelDataType rowType = typeFactory.builder()
+ .add("c1", typeFactory.createSqlType(SqlTypeName.VARCHAR,
3))
+ .add("c2", typeFactory.createSqlType(SqlTypeName.VARCHAR,
6))
+ .build();
+
+ Object[] input = {"123", "12345 6"};
+
+ assertThrowsSqlException(
+ Sql.STMT_VALIDATION_ERR,
+ "Value too long for type: VARCHAR(6)",
+ () -> buildTrimmedRow(rowType, input)
+ );
+ }
+ }
+
+ private static void expectOutputRow(RelDataType rowType, Object[] input,
Object[] expected) {
+ Object[] newRow = buildTrimmedRow(rowType, input);
+
+ assertArrayEquals(expected, newRow, "Unexpected row after
validate/trim whitespace");
+ }
+
+ private static Object[] buildTrimmedRow(RelDataType rowType, Object[]
input) {
+ List<RelDataType> columnTypes =
rowType.getFieldList().stream().map(RelDataTypeField::getType).collect(Collectors.toList());
+ RowSchema rowSchema = TypeUtils.rowSchemaFromRelTypes(columnTypes);
+
+ Object[] newRow =
TypeUtils.validateCharactersOverflowAndTrimIfPossible(rowType,
+ ArrayRowHandler.INSTANCE,
+ input,
+ () -> rowSchema
+ );
+ return newRow;
+ }
+
/**
* Checks that conversions to and from internal types is consistent.
*
