korlov42 commented on code in PR #916:
URL: https://github.com/apache/ignite-3/pull/916#discussion_r912903482
##########
modules/api/src/main/java/org/apache/ignite/schema/definition/ColumnType.java:
##########
@@ -460,8 +460,26 @@ public int hashCode() {
* Column type of variable length.
*/
public static class TemporalColumnType extends ColumnType {
- /** Default temporal type precision: microseconds. */
- public static final int DEFAULT_PRECISION = 6;
+ /**
+ * Default TIME type precision: microseconds.
+ *
+ * SQL99 part 2 section 6.1 syntax rule 30
Review Comment:
```suggestion
*
* <p>SQL99 part 2 section 6.1 syntax rule 30
```
please fix this here and below
##########
modules/api/src/main/java/org/apache/ignite/schema/definition/ColumnType.java:
##########
@@ -460,8 +460,23 @@ public int hashCode() {
* Column type of variable length.
*/
public static class TemporalColumnType extends ColumnType {
- /** Default temporal type precision: microseconds. */
- public static final int DEFAULT_PRECISION = 6;
+ /**
+ * Default TIME type precision: microseconds.
+ * SQL99 part 2 section 6.1 syntax rule 30
+ */
+ public static final int DEFAULT_TIMESTAMP_PRECISION = 6;
+
+ /**
+ * Default TIMESTAMP type precision: seconds.
+ * SQL99 part 2 section 6.1 syntax rule 30
+ */
+ public static final int DEFAULT_TIME_PRECISION = 0;
+
+ /**
+ * Max TIME precision.
+ * SQL99 part 2 section 6.1 syntax rule 31
Review Comment:
did you mean "rule 32"? It's definitely 32 rule in my PDF
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/type/IgniteTypeFactory.java:
##########
@@ -163,30 +161,72 @@ public Type getJavaClass(RelDataType type) {
}
/**
- * Gets ColumnType type for given class.
+ * Gets ColumnType type for RelDataType.
*
* @param relType Rel type.
* @return ColumnType type or null.
*/
- public ColumnType columnType(RelDataType relType) {
+ public static ColumnType relDataTypeToColumnType(RelDataType relType) {
assert relType != null;
Review Comment:
this is covered by the next assertion
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/type/IgniteTypeSystem.java:
##########
@@ -44,29 +45,69 @@ public int getMaxNumericPrecision() {
return Short.MAX_VALUE;
}
+ /** {@inheritDoc} */
+ @Override
+ public int getMaxPrecision(SqlTypeName typeName) {
+ switch (typeName) {
+ case TIME:
+ case TIME_WITH_LOCAL_TIME_ZONE:
+ case TIMESTAMP:
+ case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+ return TemporalColumnType.MAX_TIME_PRECISION;
+ default:
+ return super.getMaxPrecision(typeName);
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public int getDefaultPrecision(SqlTypeName typeName) {
+ switch (typeName) {
+ case TIMESTAMP: // DATETIME
+ return TemporalColumnType.DEFAULT_TIME_PRECISION;
Review Comment:
Could you please explain why have you decided to use _<time precision>_
here? In my opinion, we should use _<timestamp precision>_ for both TIMESTAMP
and TIMESTAMP W/ TIMEZONE types
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItMetadataTest.java:
##########
@@ -107,4 +115,118 @@ public void columnOrder() {
.columnNames("DOUBLE_C", "LONG_C", "STRING_C", "INT_C")
.check();
}
+
+ @Test
+ public void metadata() {
+ CLUSTER_NODES.get(0).sql().createSession().execute(null, "CREATE TABLE
METADATA_TABLE (" + "ID INT PRIMARY KEY, "
Review Comment:
```suggestion
sql("CREATE TABLE METADATA_TABLE (" + "ID INT PRIMARY KEY, "
```
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/type/IgniteTypeFactory.java:
##########
@@ -163,30 +161,72 @@ public Type getJavaClass(RelDataType type) {
}
/**
- * Gets ColumnType type for given class.
+ * Gets ColumnType type for RelDataType.
*
* @param relType Rel type.
* @return ColumnType type or null.
*/
- public ColumnType columnType(RelDataType relType) {
+ public static ColumnType relDataTypeToColumnType(RelDataType relType) {
assert relType != null;
+ assert relType instanceof BasicSqlType
+ || relType instanceof IntervalSqlType : "Not supported yet.";
// Implement Class->ColumnType mapping if failed.
- Type javaType = getResultClass(relType);
-
- if (javaType == byte[].class) {
- return relType.getPrecision() == PRECISION_NOT_SPECIFIED ?
ColumnType.blobOf() :
- ColumnType.blobOf(relType.getPrecision());
- } else if (javaType == String.class) {
- return relType.getPrecision() == PRECISION_NOT_SPECIFIED ?
ColumnType.string() :
- ColumnType.stringOf(relType.getPrecision());
- } else if (javaType == BigInteger.class) {
- return relType.getPrecision() == PRECISION_NOT_SPECIFIED ?
ColumnType.numberOf() :
- ColumnType.numberOf(relType.getPrecision());
- } else if (javaType == BigDecimal.class) {
- return relType.getPrecision() == PRECISION_NOT_SPECIFIED ?
ColumnType.decimalOf() :
- ColumnType.decimalOf(relType.getPrecision(),
relType.getScale());
- } else {
- return SchemaConfigurationConverter.columnType((Class<?>)
javaType);
+ switch (relType.getSqlTypeName()) {
+ case BOOLEAN:
+ throw new IllegalArgumentException("Type is not supported
yet.");
+ case TINYINT:
+ return ColumnType.INT8;
+ case SMALLINT:
+ return ColumnType.INT16;
+ case INTEGER:
+ return ColumnType.INT32;
+ case BIGINT:
+ return ColumnType.INT64;
+ case DECIMAL:
+ return relType.getPrecision() == PRECISION_NOT_SPECIFIED ?
ColumnType.decimalOf() :
Review Comment:
I believe `PRECISION_NOT_SPECIFIED` should be valid precision for varying
types only, like `VARCHAR` or `VARBINARY`. Probably we could put an assertion
here instead, WDYT?
--
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]
