twalthr commented on a change in pull request #11797:
URL: https://github.com/apache/flink/pull/11797#discussion_r413852105



##########
File path: flink-core/src/main/java/org/apache/flink/types/RowKind.java
##########
@@ -47,10 +47,69 @@
         * needs to retract the previous row first. OR it describes an 
idempotent update, i.e., an update
         * of a row that is uniquely identifiable by a key.
         */
-       UPDATE_AFTER,
+       UPDATE_AFTER("UA", (byte) 2),
 
        /**
         * Deletion operation.
         */
-       DELETE
+       DELETE("D", (byte) 3);
+
+       private final String shortString;
+
+       private final byte value;
+
+       /**
+        * Creates a {@link RowKind} enum with the given short string and byte 
value representation of
+        * the {@link RowKind}.
+        */
+       RowKind(String shortString, byte value) {
+               this.shortString = shortString;
+               this.value = value;
+       }
+
+       /**
+        * Returns a short string representation of this {@link RowKind}.
+        *
+        * <p><ul>

Review comment:
       I would remove this list. It is redundant and defined in the enum value 
already. Same for the list below.

##########
File path: flink-core/src/main/java/org/apache/flink/types/RowKind.java
##########
@@ -38,7 +38,7 @@
         * to retract the previous row first. It is useful in cases of a 
non-idempotent update, i.e., an
         * update of a row that is not uniquely identifiable by a key.
         */
-       UPDATE_BEFORE,
+       UPDATE_BEFORE("UB", (byte) 1),

Review comment:
       One comment about `UB` and `UA`: Have you thought about just calling 
them `B` and `A`? I know this might also be confusing but having a single 
character would look nicer when printed to a console otherwise rows will be 
like:
   ```
   I[]
   UB[]
   D[]
   ```

##########
File path: flink-core/src/main/java/org/apache/flink/types/RowKind.java
##########
@@ -47,10 +47,69 @@
         * needs to retract the previous row first. OR it describes an 
idempotent update, i.e., an update
         * of a row that is uniquely identifiable by a key.
         */
-       UPDATE_AFTER,
+       UPDATE_AFTER("UA", (byte) 2),
 
        /**
         * Deletion operation.
         */
-       DELETE
+       DELETE("D", (byte) 3);
+
+       private final String shortString;
+
+       private final byte value;
+
+       /**
+        * Creates a {@link RowKind} enum with the given short string and byte 
value representation of
+        * the {@link RowKind}.
+        */
+       RowKind(String shortString, byte value) {
+               this.shortString = shortString;
+               this.value = value;
+       }
+
+       /**
+        * Returns a short string representation of this {@link RowKind}.
+        *
+        * <p><ul>
+        *     <li>"I" represents {@link #INSERT}.</li>
+        *     <li>"UB" represents {@link #UPDATE_BEFORE}.</li>
+        *     <li>"UA" represents {@link #UPDATE_AFTER}.</li>
+        *     <li>"D" represents {@link #DELETE}.</li>
+        * </ul>
+        */
+       public String shortString() {
+               return shortString;
+       }
+
+       /**
+        * Returns the byte value representation of this {@link RowKind}. The 
byte value is used
+        * for serialization and deserialization.
+        *
+        * <p><ul>
+        *       <li>"0" represents {@link #INSERT}.</li>
+        *       <li>"1" represents {@link #UPDATE_BEFORE}.</li>
+        *       <li>"2" represents {@link #UPDATE_AFTER}.</li>
+        *       <li>"3" represents {@link #DELETE}.</li>
+        * </ul>
+        */
+       public byte toByteValue() {
+               return value;
+       }
+
+       /**
+        * Creates a {@link RowKind} from the given byte value. Each {@link 
RowKind} has a byte
+        * value representation.
+        *
+        * @see #toByteValue() for mapping of byte value and {@link RowKind}.
+        */
+       public static RowKind fromByteValue(byte value) {
+               switch (value) {
+                       case 0: return INSERT;
+                       case 1: return UPDATE_BEFORE;
+                       case 2: return UPDATE_AFTER;
+                       case 3: return DELETE;
+                       default: throw new UnsupportedOperationException(
+                               "Do not support to parse byte value " + value + 
" to RowKind.");

Review comment:
       nit: `Unsupported byte value ' + value + ' for row kind.`

##########
File path: flink-core/src/main/java/org/apache/flink/types/RowKind.java
##########
@@ -47,10 +47,69 @@
         * needs to retract the previous row first. OR it describes an 
idempotent update, i.e., an update
         * of a row that is uniquely identifiable by a key.
         */
-       UPDATE_AFTER,
+       UPDATE_AFTER("UA", (byte) 2),
 
        /**
         * Deletion operation.
         */
-       DELETE
+       DELETE("D", (byte) 3);
+
+       private final String shortString;
+
+       private final byte value;
+
+       /**
+        * Creates a {@link RowKind} enum with the given short string and byte 
value representation of
+        * the {@link RowKind}.
+        */
+       RowKind(String shortString, byte value) {

Review comment:
       Thanks for pinging me about this. Sounds good to me. Thanks for 
performing benchmarks. I added some comments.




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to