korlov42 commented on code in PR #912:
URL: https://github.com/apache/ignite-3/pull/912#discussion_r911755798
##########
modules/schema/src/test/java/org/apache/ignite/internal/schema/ColumnDefinitionTest.java:
##########
@@ -43,6 +43,7 @@ public void compareColumns() {
new Column("A", DATE, false),
new Column("AD", STRING, false),
new Column("AA", STRING, false),
+ new Column(-100, "J", STRING, false)
Review Comment:
why do we need this particular change?
##########
modules/api/src/main/java/org/apache/ignite/schema/definition/ColumnType.java:
##########
@@ -173,6 +173,38 @@ public static DecimalColumnType decimalOf() {
);
}
+ /**
+ * Returns A time-based amount of time with the default time precision.
Review Comment:
this is actually how it is described in javadoc of built-in Period.class: `A
time-based amount of time, such as '34.5 seconds'.`
##########
modules/client/src/test/java/org/apache/ignite/client/ClientTupleTest.java:
##########
@@ -149,7 +151,9 @@ public void testTypedGetters() {
new ClientColumn("TIME", ClientDataType.TIME, false, false, 9),
new ClientColumn("DATE", ClientDataType.DATE, false, false,
10),
new ClientColumn("DATETIME", ClientDataType.DATETIME, false,
false, 11),
- new ClientColumn("TIMESTAMP", ClientDataType.TIMESTAMP, false,
false, 12)
+ new ClientColumn("TIMESTAMP", ClientDataType.TIMESTAMP, false,
false, 12),
+ new ClientColumn("DURATION", ClientDataType.DURATION, false,
false, 13),
+ new ClientColumn("PERIOD", ClientDataType.PERIOD, false,
false, 14)
});
Review Comment:
could you please add a validation that size of created schema equals to
count of types defined in ClientDataType (we need to detect this automatically
somehow)?
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/NativeTypes.java:
##########
@@ -190,6 +190,37 @@ public static NativeType timestamp() {
return
TemporalNativeType.timestamp(ColumnType.TemporalColumnType.DEFAULT_PRECISION);
}
+ /**
+ * Creates DURATION type.
+ *
+ * @see NativeTypeSpec#DURATION
+ * @param precision Fractional seconds meaningful digits. Allowed values
are 0-9 for second to nanosecond precision.
+ * @return Native type.
+ */
+ public static NativeType duration(int precision) {
+ return TemporalNativeType.duration(precision);
+ }
+
+ /**
+ * Creates DURATION type with default precision.
+ *
+ * @see NativeTypeSpec#DURATION
+ * @return Native type.
+ */
+ public static NativeType duration() {
+ return
TemporalNativeType.duration(ColumnType.TemporalColumnType.DEFAULT_PRECISION);
+ }
+
+ /**
+ * Creates PERIOD type with default precision.
+ *
+ * @see NativeTypeSpec#PERIOD
+ * @return Native type.
+ */
+ public static NativeType period() {
Review Comment:
does it make sense to provide a way to specify precision for period?
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java:
##########
@@ -703,6 +715,58 @@ public RowAssembler appendTimestamp(Instant val) throws
SchemaMismatchException
return this;
}
+ /**
+ * Appends Duration value for the current column to the chunk.
+ *
+ * @param val Column value.
+ * @return {@code this} for chaining.
+ * @throws SchemaMismatchException If a value doesn't match the current
column type.
+ */
+ public RowAssembler appendDuration(Duration val) throws
SchemaMismatchException {
+ checkType(NativeTypeSpec.DURATION);
+
+ TemporalNativeType type = (TemporalNativeType)
curCols.column(curCol).type();
+
+ long seconds = val.getSeconds();
+ int nanos = TemporalTypesHelper.normalizeNanos(val.getNano(),
type.precision());
+
+ buf.putLong(curOff, seconds);
+
+ if (type.precision() != 0) {
+ // Write only meaningful bytes.
+ buf.putInt(curOff + 8, nanos);
+ }
+
+ shiftColumn(type.sizeInBytes());
+
+ return this;
+ }
+
+ /**
+ * Appends Period value for the current column to the chunk.
+ *
+ * @param val Column value.
+ * @return {@code this} for chaining.
+ * @throws SchemaMismatchException If a value doesn't match the current
column type.
+ */
+ public RowAssembler appendPeriod(Period val) throws
SchemaMismatchException {
+ checkType(NativeTypeSpec.PERIOD);
+
+ TemporalNativeType type = (TemporalNativeType)
curCols.column(curCol).type();
+
+ int years = val.getYears();
+ int months = val.getMonths();
+ int days = val.getDays();
+
+ buf.putLong(curOff, years);
Review Comment:
why do you use `putLong` when the value actually of type int?
##########
modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverterTest.java:
##########
@@ -96,7 +96,9 @@ public void createRegistry() throws ExecutionException,
InterruptedException {
SchemaBuilders.column("COL2",
ColumnType.DOUBLE).build(),
SchemaBuilders.column("A", ColumnType.INT8).build(),
SchemaBuilders.column("B", ColumnType.INT8).build(),
- SchemaBuilders.column("C", ColumnType.INT8).build()
+ SchemaBuilders.column("C", ColumnType.INT8).build(),
+ SchemaBuilders.column("DUR",
ColumnType.duration()).build(),
+ SchemaBuilders.column("PER",
ColumnType.period()).build()
Review Comment:
could you please rewrite tests here and below in order to automatically
verify all types from ColumnTypeSpec/NativeTypeSpec (it is OK to have ignore
list though)?
##########
modules/api/src/main/java/org/apache/ignite/schema/definition/ColumnType.java:
##########
@@ -173,6 +173,38 @@ public static DecimalColumnType decimalOf() {
);
}
+ /**
+ * Returns A time-based amount of time with the default time precision.
+ *
+ * @see TemporalColumnType#DEFAULT_PRECISION
+ * @see #duration(int)
+ */
+ public static TemporalColumnType duration() {
+ return new TemporalColumnType(ColumnTypeSpec.DURATION,
TemporalColumnType.DEFAULT_PRECISION);
+ }
+
+ /**
+ * Returns a time-based amount of time with the default time precision.
+ *
+ * <p>Precision is a number of digits in fractional seconds part, from 0 -
whole seconds precision up to 9 - nanoseconds precision.
+ *
+ * @param precision The number of digits in fractional seconds part.
Accepted values are in range [0-9].
+ * @return Native type.
+ * @throws IllegalArgumentException If precision value was invalid.
+ */
+ public static TemporalColumnType duration(int precision) {
+ return new TemporalColumnType(ColumnTypeSpec.DURATION, precision);
+ }
+
+ /**
+ * Returns A date-based amount of time with the default time precision.
+ *
+ * @see TemporalColumnType#DEFAULT_PRECISION
+ */
+ public static TemporalColumnType period() {
Review Comment:
does it make sense to provide a factory method accepting precision as
argument?
##########
modules/client-common/src/main/java/org/apache/ignite/internal/jdbc/proto/event/JdbcColumnMeta.java:
##########
@@ -390,6 +392,10 @@ private static String typeName(String cls) {
return "BINARY";
} else if (Time.class.getName().equals(cls)) {
return "TIME";
+ } else if (Duration.class.getName().equals(cls)) {
Review Comment:
could you please provide a test to validate that metadata of all types are
properly propagated?
--
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]
