xtern commented on code in PR #3627:
URL: https://github.com/apache/ignite-3/pull/3627#discussion_r1571197859
##########
modules/core/src/main/java/org/apache/ignite/internal/util/io/IgniteDataOutput.java:
##########
@@ -129,6 +139,86 @@ public interface IgniteDataOutput extends DataOutput {
*/
void writeCharArray(char[] arr) throws IOException;
+ /**
+ * Writes big integer.
+ *
+ * @param val Big integer.
+ * @throws IOException In case of error.
+ */
+ void writeBigInteger(BigInteger val) throws IOException;
+
+ /**
+ * Writes decimal.
+ *
+ * @param val Decimal.
Review Comment:
```suggestion
* Writes big decimal.
*
* @param val Big decimal.
```
for consistency with `readBigDecimal` :sunglasses:
##########
modules/core/src/test/java/org/apache/ignite/internal/util/io/IgniteUnsafeDataInputOutputByteOrderTest.java:
##########
@@ -237,4 +249,101 @@ public void testDoubleArray() throws Exception {
assertArrayEquals(arr, in.readDoubleArray(ARR_LEN), 0);
}
+
+ @Test
+ public void testLocalTime() throws IOException {
+ LocalTime val = LocalTime.of(RND.nextInt(24), RND.nextInt(60),
RND.nextInt(60), RND.nextInt(10000));
+
+ out.writeLocalTime(val);
+
+ assertEquals(val, in.readLocalTime());
+ }
+
+ @Test
+ public void testLocalDate() throws IOException {
+ LocalDate val = LocalDate.of(RND.nextInt(500) + 1900, RND.nextInt(12)
+ 1, 1 + RND.nextInt(27));
+
+ out.writeLocalDate(val);
+
+ assertEquals(val, in.readLocalDate());
+ }
+
+ @Test
+ public void testLocalDateTime() throws IOException {
+ LocalTime time = LocalTime.of(RND.nextInt(24), RND.nextInt(60),
RND.nextInt(60), RND.nextInt(10000));
+ LocalDate date = LocalDate.of(RND.nextInt(500) + 1900, RND.nextInt(12)
+ 1, 1 + RND.nextInt(27));
Review Comment:
```suggestion
LocalDate date = LocalDate.of(RND.nextInt(4000) - 1000,
RND.nextInt(12) + 1, 1 + RND.nextInt(27));
```
##########
modules/core/src/test/java/org/apache/ignite/internal/util/io/IgniteUnsafeDataInputOutputByteOrderTest.java:
##########
@@ -237,4 +249,101 @@ public void testDoubleArray() throws Exception {
assertArrayEquals(arr, in.readDoubleArray(ARR_LEN), 0);
}
+
+ @Test
+ public void testLocalTime() throws IOException {
+ LocalTime val = LocalTime.of(RND.nextInt(24), RND.nextInt(60),
RND.nextInt(60), RND.nextInt(10000));
+
+ out.writeLocalTime(val);
+
+ assertEquals(val, in.readLocalTime());
+ }
+
+ @Test
+ public void testLocalDate() throws IOException {
+ LocalDate val = LocalDate.of(RND.nextInt(500) + 1900, RND.nextInt(12)
+ 1, 1 + RND.nextInt(27));
+
+ out.writeLocalDate(val);
+
+ assertEquals(val, in.readLocalDate());
+ }
+
+ @Test
+ public void testLocalDateTime() throws IOException {
+ LocalTime time = LocalTime.of(RND.nextInt(24), RND.nextInt(60),
RND.nextInt(60), RND.nextInt(10000));
+ LocalDate date = LocalDate.of(RND.nextInt(500) + 1900, RND.nextInt(12)
+ 1, 1 + RND.nextInt(27));
+ LocalDateTime val = LocalDateTime.of(date, time);
+
+ out.writeLocalDateTime(val);
+
+ assertEquals(val, in.readLocalDateTime());
+ }
+
+ @Test
+ public void testInstant() throws IOException {
+ Instant val = Instant.ofEpochMilli(Math.abs(RND.nextLong()));
Review Comment:
```suggestion
Instant val = Instant.ofEpochMilli(RND.nextLong());
```
With Math.abs, this will only cover dates starting from 1970.
##########
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/storage/CatalogEntrySerializationTest.java:
##########
@@ -45,20 +59,36 @@
import
org.apache.ignite.internal.catalog.storage.serialization.MarshallableEntryType;
import
org.apache.ignite.internal.catalog.storage.serialization.UpdateLogMarshallerImpl;
import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
-import org.apache.ignite.internal.tostring.S;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypes;
+import org.apache.ignite.internal.util.io.IgniteUnsafeDataInput;
+import org.apache.ignite.internal.util.io.IgniteUnsafeDataOutput;
import org.apache.ignite.sql.ColumnType;
import org.assertj.core.api.BDDAssertions;
import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.EnumSource.Mode;
+import org.junit.jupiter.params.provider.MethodSource;
/**
* Tests to verify catalog storage entries serialization.
*/
public class CatalogEntrySerializationTest extends BaseIgniteAbstractTest {
+ private static final long seed = System.nanoTime();
+
+ private static final Random RND = new Random(seed);
+
private final UpdateLogMarshallerImpl marshaller = new
UpdateLogMarshallerImpl();
+ @BeforeEach
+ public void setup() {
+ log.info("Seed: {}", seed);
Review Comment:
Could you add the same initialization to
`IgniteUnsafeDataInputOutputByteOrderTest` if it doesn't bother you?
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DefaultValue.java:
##########
@@ -68,9 +92,28 @@ public Type type() {
return type;
}
+ /** Serializes this default value or {@code null}. */
+ public abstract void writeTo(IgniteDataOutput os) throws IOException;
+
+ /** Reads default value or {@code null}. */
+ public static @Nullable DefaultValue readFrom(IgniteDataInput in) throws
IOException {
+ int typeId = in.readByte();
+ if (typeId == Type.NO_DEFAULT) {
+ return null;
+ } else if (typeId == Type.CONSTANT.typeId) {
+ Object val = readValue(in);
+ return new ConstantValue(val);
+ } else if (typeId == Type.FUNCTION_CALL.typeId) {
+ String functionName = in.readUTF();
+ return new FunctionCall(functionName);
+ } else {
+ throw new IllegalArgumentException("Unexpected type: " + typeId);
Review Comment:
may be use switch/case?
##########
modules/core/src/test/java/org/apache/ignite/internal/util/io/IgniteUnsafeDataInputOutputByteOrderTest.java:
##########
@@ -237,4 +249,101 @@ public void testDoubleArray() throws Exception {
assertArrayEquals(arr, in.readDoubleArray(ARR_LEN), 0);
}
+
+ @Test
+ public void testLocalTime() throws IOException {
+ LocalTime val = LocalTime.of(RND.nextInt(24), RND.nextInt(60),
RND.nextInt(60), RND.nextInt(10000));
+
+ out.writeLocalTime(val);
+
+ assertEquals(val, in.readLocalTime());
+ }
+
+ @Test
+ public void testLocalDate() throws IOException {
+ LocalDate val = LocalDate.of(RND.nextInt(500) + 1900, RND.nextInt(12)
+ 1, 1 + RND.nextInt(27));
Review Comment:
```suggestion
LocalDate val = LocalDate.of(RND.nextInt(4000) - 1000,
RND.nextInt(12) + 1, 1 + RND.nextInt(27));
```
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DefaultValue.java:
##########
@@ -68,9 +92,28 @@ public Type type() {
return type;
}
+ /** Serializes this default value or {@code null}. */
+ public abstract void writeTo(IgniteDataOutput os) throws IOException;
+
+ /** Reads default value or {@code null}. */
+ public static @Nullable DefaultValue readFrom(IgniteDataInput in) throws
IOException {
Review Comment:
First, there is a "static" version of `writeTo` somewhere below (hard to
understand).
Secondly, from my point of view it might be worth implementing them
identically... I understand that this is more like structured programming and
not OOP, but it seems in this particular case it will be simpler, shorter and
clearer.
```
/** Writes the given default value into output. */
public static void writeTo(@Nullable DefaultValue val, IgniteDataOutput out)
throws IOException {
if (val == null) {
out.writeByte(Type.NO_DEFAULT);
} else {
out.writeByte(val.type.typeId);
if (val.type == Type.CONSTANT) {
ConstantValue val0 = (ConstantValue) val;
writeValue(val0.columnType, val0.value, out);
} else {
assert val instanceof FunctionCall;
out.writeUTF(((FunctionCall) val).functionName());
}
}
}
```
wdyt?
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DefaultValue.java:
##########
@@ -51,10 +65,20 @@ public static DefaultValue constant(@Nullable Object value)
{
/** Types of the defaults. */
public enum Type {
/** Default is specified as a constant. */
- CONSTANT,
+ CONSTANT(0),
/** Default is specified as a call to a function. */
- FUNCTION_CALL
+ FUNCTION_CALL(1);
+
+ /** Represents absent of default value ({@code null}). */
+ private static final int NO_DEFAULT = -1;
+
+ /** type id used by serialization. */
Review Comment:
```suggestion
/** Type id used by serialization. */
```
--
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]