You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/08/16 12:59:51 UTC
[GitHub] [ignite-3] ptupitsyn commented on a change in pull request #284: IGNITE-15186 JdbcStatement and resultSet
ptupitsyn commented on a change in pull request #284:
URL: https://github.com/apache/ignite-3/pull/284#discussion_r689506274
##########
File path: modules/client-common/pom.xml
##########
@@ -95,6 +95,11 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-calcite</artifactId>
Review comment:
`ignite-client-common` is for sharing serialization logic between `ignite-client-handler` and `ignite-client`. We should not have such a heavy reference here, most likely it should be moved to `ignite-client-handler`.
##########
File path: modules/client-common/src/main/java/org/apache/ignite/client/proto/ClientMessagePacker.java
##########
@@ -347,6 +347,30 @@ public ClientMessagePacker packBitSet(BitSet val) {
throw new UnsupportedOperationException("TODO: IGNITE-15163");
}
+ /**
+ * Writes an integer array.
+ *
+ * @param arr Integer array value.
+ * @return This instance.
+ * @throws IOException when underlying output throws IOException.
+ */
+ public ClientMessagePacker packIntArray(int[] arr) throws IOException {
+ assert !closed : "Packer is closed";
+
+ if (arr == null) {
+ packInt(0);
+
+ return this;
+ }
+
+ packInt(arr.length);
Review comment:
```suggestion
packArrayHeader(arr.length);
```
##########
File path: modules/client-common/src/main/java/org/apache/ignite/client/proto/ClientMessageUnpacker.java
##########
@@ -413,6 +433,109 @@ public Object unpackObject(int dataType) {
throw new IgniteException("Unknown client data type: " + dataType);
}
+ /**
+ * Packs an object.
+ *
+ * @return Object array.
+ * @throws IllegalStateException in case of unexpected value type.
+ */
+ public Object[] unpackObjectArray() {
+ assert refCnt > 0 : "Unpacker is closed";
+
+ if (tryUnpackNil())
+ return null;
+
+ int size = unpackInt();
Review comment:
`unpackArrayHeader`
##########
File path: modules/client-common/src/main/java/org/apache/ignite/client/proto/ClientMessageUnpacker.java
##########
@@ -413,6 +433,109 @@ public Object unpackObject(int dataType) {
throw new IgniteException("Unknown client data type: " + dataType);
}
+ /**
+ * Packs an object.
+ *
+ * @return Object array.
+ * @throws IllegalStateException in case of unexpected value type.
+ */
+ public Object[] unpackObjectArray() {
+ assert refCnt > 0 : "Unpacker is closed";
+
+ if (tryUnpackNil())
+ return null;
+
+ int size = unpackInt();
+
+ if (size == 0)
+ return new Object[size];
+
+ Object[] args = new Object[size];
+
+ for (int i = 0; i < size; i++) {
+ if (tryUnpackNil())
+ continue;
+
+ ImmutableValue v = unpackValue();
+
+ switch (v.getValueType()) {
+ case BOOLEAN:
+ args[i] = v.asBooleanValue().getBoolean();
+
+ break;
+ case INTEGER:
+ args[i] = extractNumberValue(v.asIntegerValue());
+
+ break;
+ case FLOAT:
+ args[i] = v.asFloatValue().toDouble();
+
+ break;
+ case STRING:
+ args[i] = v.asStringValue().asString();
+
+ break;
+ case BINARY:
+ args[i] = v.asBinaryValue().asByteArray();
+
+ break;
+ case EXTENSION: {
+ ExtensionValue val = v.asExtensionValue();
+
+ if (val.getType() == ClientMsgPackType.UUID) {
+ args[i] = bytesToUUID(val);
+ continue;
+ } else
+ throw new IllegalStateException("Unexpected extended value type: " + val.getType());
+ }
+ default:
+ throw new IllegalStateException("Unexpected value type: " + v.getValueType());
+ }
+ }
+ return args;
+ }
+
+ /**
+ * Extracts number value according to value range.
+ *
+ * @param iv IntegerValue.
+ * @return Numeric java object selected based on the value range.
+ */
+ private Object extractNumberValue(IntegerValue iv) {
Review comment:
This logic may result in a weird behavior where different types are returned depending on the value. Especially if we return this to the user. This is why `unpackObject` method takes a type code - I suggest using the same approach here.
##########
File path: modules/client-common/src/main/java/org/apache/ignite/client/proto/ClientMessagePacker.java
##########
@@ -347,6 +347,30 @@ public ClientMessagePacker packBitSet(BitSet val) {
throw new UnsupportedOperationException("TODO: IGNITE-15163");
}
+ /**
+ * Writes an integer array.
+ *
+ * @param arr Integer array value.
+ * @return This instance.
+ * @throws IOException when underlying output throws IOException.
+ */
+ public ClientMessagePacker packIntArray(int[] arr) throws IOException {
+ assert !closed : "Packer is closed";
+
+ if (arr == null) {
+ packInt(0);
Review comment:
```suggestion
packNil();
```
##########
File path: modules/client-common/src/main/java/org/apache/ignite/client/proto/ClientMessageUnpacker.java
##########
@@ -413,6 +433,109 @@ public Object unpackObject(int dataType) {
throw new IgniteException("Unknown client data type: " + dataType);
}
+ /**
+ * Packs an object.
+ *
+ * @return Object array.
+ * @throws IllegalStateException in case of unexpected value type.
+ */
+ public Object[] unpackObjectArray() {
+ assert refCnt > 0 : "Unpacker is closed";
+
+ if (tryUnpackNil())
+ return null;
+
+ int size = unpackInt();
+
+ if (size == 0)
+ return new Object[size];
Review comment:
`ArrayUtils.OBJECT_EMPTY_ARRAY`
##########
File path: modules/client-common/src/main/java/org/apache/ignite/client/proto/ClientMessagePacker.java
##########
@@ -388,6 +412,65 @@ public ClientMessagePacker packObject(Object val) {
throw new UnsupportedOperationException("Unsupported type, can't serialize: " + val.getClass());
}
+ /**
+ * Packs an array of different objects.
+ *
+ * @param args Object array.
+ * @return This instance.
+ * @throws IOException when underlying output throws IOException.
+ */
+ public ClientMessagePacker packObjectArray(Object[] args) throws IOException {
+ assert !closed : "Packer is closed";
+
+ if (args == null) {
+ packNil();
+
+ return this;
+ }
+
+ packInt(args.length);
Review comment:
`packArrayHeader`
##########
File path: modules/client-common/src/main/java/org/apache/ignite/client/proto/ClientMessageUnpacker.java
##########
@@ -413,6 +433,109 @@ public Object unpackObject(int dataType) {
throw new IgniteException("Unknown client data type: " + dataType);
}
+ /**
+ * Packs an object.
+ *
+ * @return Object array.
+ * @throws IllegalStateException in case of unexpected value type.
+ */
+ public Object[] unpackObjectArray() {
+ assert refCnt > 0 : "Unpacker is closed";
+
+ if (tryUnpackNil())
+ return null;
+
+ int size = unpackInt();
+
+ if (size == 0)
+ return new Object[size];
+
+ Object[] args = new Object[size];
+
+ for (int i = 0; i < size; i++) {
+ if (tryUnpackNil())
+ continue;
+
+ ImmutableValue v = unpackValue();
Review comment:
We can avoid extra allocations here by switching on `getNextFormat()` and then calling corresponding `unpackX` method.
##########
File path: modules/client-common/src/main/java/org/apache/ignite/client/proto/ClientMessageUnpacker.java
##########
@@ -413,6 +433,109 @@ public Object unpackObject(int dataType) {
throw new IgniteException("Unknown client data type: " + dataType);
}
+ /**
+ * Packs an object.
+ *
+ * @return Object array.
+ * @throws IllegalStateException in case of unexpected value type.
+ */
+ public Object[] unpackObjectArray() {
+ assert refCnt > 0 : "Unpacker is closed";
+
+ if (tryUnpackNil())
+ return null;
+
+ int size = unpackInt();
+
+ if (size == 0)
+ return new Object[size];
+
+ Object[] args = new Object[size];
+
+ for (int i = 0; i < size; i++) {
+ if (tryUnpackNil())
+ continue;
+
+ ImmutableValue v = unpackValue();
+
+ switch (v.getValueType()) {
+ case BOOLEAN:
+ args[i] = v.asBooleanValue().getBoolean();
+
+ break;
+ case INTEGER:
+ args[i] = extractNumberValue(v.asIntegerValue());
+
+ break;
+ case FLOAT:
+ args[i] = v.asFloatValue().toDouble();
+
+ break;
+ case STRING:
+ args[i] = v.asStringValue().asString();
+
+ break;
+ case BINARY:
+ args[i] = v.asBinaryValue().asByteArray();
+
+ break;
+ case EXTENSION: {
+ ExtensionValue val = v.asExtensionValue();
+
+ if (val.getType() == ClientMsgPackType.UUID) {
+ args[i] = bytesToUUID(val);
+ continue;
+ } else
+ throw new IllegalStateException("Unexpected extended value type: " + val.getType());
+ }
+ default:
+ throw new IllegalStateException("Unexpected value type: " + v.getValueType());
+ }
+ }
+ return args;
+ }
+
+ /**
+ * Extracts number value according to value range.
+ *
+ * @param iv IntegerValue.
+ * @return Numeric java object selected based on the value range.
+ */
+ private Object extractNumberValue(IntegerValue iv) {
+ if (iv.isInByteRange())
+ return iv.asByte();
+ if (iv.isInShortRange())
+ return iv.asShort();
+ if (iv.isInIntRange())
+ return iv.asInt();
+ else if (iv.isInLongRange())
+ return iv.asLong();
+
+ return iv.toBigInteger();
+ }
+
+ /**
+ * Reads an UUID.
+ *
+ * @param val ExtensionValue.
+ * @return UUID value.
+ * @throws MessageTypeException when type is not UUID.
+ * @throws MessageSizeException when size is not correct.
+ */
+ private Object bytesToUUID(ExtensionValue val) {
Review comment:
This duplication of `unpackUuid` won't be necessary if we use `getNextFormat()` as noted above.
##########
File path: modules/client-common/src/main/java/org/apache/ignite/client/proto/ClientMessageUnpacker.java
##########
@@ -361,6 +363,24 @@ public BitSet unpackBitSet() {
throw new UnsupportedOperationException("TODO: IGNITE-15163");
}
+ /**
+ * Reads an integer array.
+ *
+ * @return Integer array.
+ */
+ public int[] unpackIntArray() {
+ assert refCnt > 0 : "Unpacker is closed";
+
+ int size = unpackInt();
Review comment:
`unpackArrayHeader`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org