ptupitsyn commented on code in PR #916:
URL: https://github.com/apache/ignite-3/pull/916#discussion_r912049171
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/type/IgniteTypeSystem.java:
##########
@@ -44,29 +45,80 @@ 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;
+ case TIMESTAMP_WITH_LOCAL_TIME_ZONE: // TIMESTAMP
+ return TemporalColumnType.DEFAULT_TIMESTAMP_PRECISION;
+ default:
+ return super.getDefaultPrecision(typeName);
+ }
+ }
+
/** {@inheritDoc} */
@Override
public RelDataType deriveSumType(RelDataTypeFactory typeFactory,
RelDataType argumentType) {
RelDataType sumType;
+ // This method is used to derive types for aggregate functions only.
+ // We allow type widening for aggregates to prevent unwanted number
overflow.
+ //
+ // SQL`99 part 2 section 9.3 syntax rule 3:
+ // Shortly, the standard says both, the return type and the argument
type, must be of the same kind,
+ // but doesn't restrict them being exactly of the same type.
if (argumentType instanceof BasicSqlType) {
switch (argumentType.getSqlTypeName()) {
case INTEGER:
case TINYINT:
case SMALLINT:
- sumType = typeFactory.createSqlType(SqlTypeName.BIGINT);
+ sumType = typeFactory.createTypeWithNullability(
+ // TODO: can we cache the type?
Review Comment:
TODOs should have tickets.
##########
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.");
Review Comment:
Do we need a ticket for this?
--
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]
