alex-plekhanov commented on a change in pull request #9826:
URL: https://github.com/apache/ignite/pull/9826#discussion_r825797521
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheTableDescriptorImpl.java
##########
@@ -309,7 +308,11 @@ else if (affFields.isEmpty())
Object dfltVal = TypeUtils.toInternal(dataCtx, desc.defaultValue());
- return rexBuilder.makeLiteral(dfltVal, desc.logicalType(typeFactory),
false);
+ if (dfltVal instanceof UUID)
+ return rexBuilder.makeCast(typeFactory.createUUIDType(),
+ rexBuilder.makeLiteral(dfltVal.toString(),
desc.logicalType(typeFactory), false));
+
+ return rexBuilder.makeLiteral(dfltVal, desc.logicalType(typeFactory),
true);
Review comment:
Let's move it to the new method of `TypeUtils` class (for example,
`toRexLiteral`)
##########
File path:
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDmlIntegrationTest.java
##########
@@ -493,6 +497,9 @@ public void testInsertDefaultValue() {
checkWrongDefault("INTERVAL MONTHS", "INTERVAL '10' DAYS");
checkWrongDefault("VARBINARY", "'10'");
checkWrongDefault("VARBINARY", "10");
+ checkWrongDefault("VARBINARY", "10");
+ checkWrongDefault("VARBINARY", "10");
Review comment:
Duplicated
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/Accumulators.java
##########
@@ -873,7 +887,7 @@ private DecimalMinMax(boolean min) {
}
/** */
- private static class VarBinaryMinMax implements Accumulator {
+ private static class VarBinaryMinMax<T extends Comparable<T>> implements
Accumulator {
Review comment:
Generalized class is not used. Let's create something like
`ComparableMinMax` and create static instances of this class
`VARBINARY_MIN_FACTORY`, `VARBINARY_MAX_FACTORY`, `UUID_MIN_FACTORY`,
`UUID_MAX_FACTORY`
##########
File path:
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/DataTypesTest.java
##########
@@ -32,6 +33,77 @@
* Test SQL data types.
*/
public class DataTypesTest extends AbstractBasicIntegrationTest {
+ /** Tests UUID without index. */
+ @Test
+ public void testUUIDWithoutIndex() {
+ testUUID(false);
+ }
+
+ /** Tests UUID with the index. */
+ @Test
+ public void testUUIDWithIndex() {
+ testUUID(true);
+ }
+
+ /** Tests UUID type. */
+ private void testUUID(boolean indexed) {
+ try {
+ executeSql("CREATE TABLE t(id INT, name VARCHAR(255), uid UUID,
primary key (id))");
+
+ if (indexed)
+ executeSql("CREATE INDEX uuid_idx ON t (uid);");
+
+ executeSql("INSERT INTO t VALUES (1,
'fd10556e-fc27-4a99-b5e4-89b8344cb3ce', " +
+ "'9e120341-627f-32be-8393-58b5d655b751')");
+ // Name == UUID
+ executeSql("INSERT INTO t VALUES (2,
'123e4567-e89b-12d3-a456-426614174000', " +
+ "'123e4567-e89b-12d3-a456-426614174000')");
+
+ assertQuery("SELECT * FROM t")
+ .returns(1, "fd10556e-fc27-4a99-b5e4-89b8344cb3ce",
UUID.fromString("9e120341-627f-32be-8393-58b5d655b751"))
+ .returns(2, "123e4567-e89b-12d3-a456-426614174000",
UUID.fromString("123e4567-e89b-12d3-a456-426614174000"))
+ .check();
+
+ assertQuery("SELECT * FROM t WHERE uid <
'123e4567-e89b-12d3-a456-426614174000'")
+ .returns(1, "fd10556e-fc27-4a99-b5e4-89b8344cb3ce",
UUID.fromString("9e120341-627f-32be-8393-58b5d655b751"))
+ .check();
+
+ assertQuery("SELECT * FROM t WHERE
'123e4567-e89b-12d3-a456-426614174000' > uid")
+ .returns(1, "fd10556e-fc27-4a99-b5e4-89b8344cb3ce",
UUID.fromString("9e120341-627f-32be-8393-58b5d655b751"))
+ .check();
+
+ assertQuery("SELECT * FROM t WHERE name = uid")
+ .returns(2, "123e4567-e89b-12d3-a456-426614174000",
UUID.fromString("123e4567-e89b-12d3-a456-426614174000"))
+ .check();
+
+ assertQuery("SELECT * FROM t WHERE name != uid")
+ .returns(1, "fd10556e-fc27-4a99-b5e4-89b8344cb3ce",
UUID.fromString("9e120341-627f-32be-8393-58b5d655b751"))
+ .check();
+
+ assertQuery("SELECT count(*), uid FROM t group by uid order by
uid")
Review comment:
SUM and AVG is confusing for UUID, I think it's better to throw error
like 'not supported' (the same as for VARCHAR type)
--
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]
