[GitHub] [ignite-3] ptupitsyn commented on a diff in pull request #2013: [POC] User object serialization

["ptupitsyn (via GitHub)" <gi...@apache.org>]

ptupitsyn commented on code in PR #2013:
URL: https://github.com/apache/ignite-3/pull/2013#discussion_r1183230969


##########
modules/platforms/dotnet/Apache.Ignite/Internal/Proto/BinaryTuple/BinaryTupleReader.cs:
##########
@@ -412,6 +413,17 @@ public bool IsNull(int index)
             var s => throw GetInvalidLengthException(index, 12, s.Length)
         };
 
+        /// <summary>
+        /// Deserialize object from JSON.
+        /// </summary>
+        /// <param name="index">Index.</param>
+        /// <typeparam name="T">Value type.</typeparam>
+        /// <returns>Value.</returns>
+        public T? GetFromJson<T>(int index) => Seek(index) switch

Review Comment:
   This logic should not be a part of `BinaryTupleReader`.



##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/compute/ClientComputeExecuteRequest.java:
##########
@@ -66,7 +69,14 @@ public static CompletableFuture<Void> process(
      * @param in Unpacker.
      * @return Args array.
      */
-    static Object[] unpackArgs(ClientMessageUnpacker in) {
-        return in.unpackObjectArrayFromBinaryTuple();
+    static Object[] unpackArgs(ClientMessageUnpacker in, String jobClassName) {

Review Comment:
   Ideally we should not unpack arguments if we are not on the target node at the moment.



##########
modules/api/src/main/java/org/apache/ignite/sql/ColumnType.java:
##########
@@ -90,7 +90,10 @@ public enum ColumnType {
     NUMBER,
 
     /** Null. */
-    NULL;
+    NULL,
+
+    /** Binary. */
+    BINARY;

Review Comment:
   I don't think we should add a new SQL column type for this. Let's use existing `BYTE_ARRAY`.



-- 
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