[GitHub] [drill] paul-rogers commented on a change in pull request #1870: DRILL-7359: Add support for DICT type in RowSet Framework

[GitBox <gi...@apache.org>]

paul-rogers commented on a change in pull request #1870: DRILL-7359: Add support for DICT type in RowSet Framework
URL: https://github.com/apache/drill/pull/1870#discussion_r352737013
 
 

 ##########
 File path: exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/reader/UnionReaderImpl.java
 ##########
 @@ -268,4 +274,51 @@ public String getAsString() {
     }
     return requireReader(type).getAsString();
   }
+
+  private UnionReaderImpl getNullReader() {
+    AbstractObjectReader[] nullVariants = new AbstractObjectReader[variants.length];
+    for (int i = 0; i < variants.length; i++) {
+      nullVariants[i] = variants[i].createNullReader();
+    }
+    return new NullUnionReader(schema(), unionAccessor, nullVariants);
+  }
+
+  private static class NullUnionReader extends UnionReaderImpl {
+
+    private NullUnionReader(ColumnMetadata metadata, VectorAccessor va, AbstractObjectReader[] variants) {
 
 Review comment:
   We may want to define our semantics here. If I understand your description, we have:
   
   * The entire `DICT` is `NULL` (is this allowed)?
   * The `DICT` has an entry for key `x`, but the value is null.
   * The `DICT` has an entry for the `NULL` key.
   * The `DICT` has no entry for the key `x`, meaning that the value is implicitly `NULL`.
   
   We have similar situations with the `UNION` type, perhaps we can learn from that:
   
   * The entire `UNION` can be `NULL`.
   * The `UNION` can have an entry for, say, `VARCHAR`, but the `VARCHAR` is `NULL`.
   * The `UNION` does not have an entry for a given type (say, `INT`), so the value is implicitly `NULL`. (Although we don't allow access to such a type.)
   
   My suggestion would be that we use the `isNull()` method for all the `NULL` cases:
   
   * If the `DICT` is `NULL` for a row, then `isNull()` returns true for the `DICT` reader.
   * If the `DICT` has a `NULL` key, then the key `isNull()` for the key returns true.
   * If the `DICT` has a key, but the value is `NULL`, then `isNull()` for the value returns true.
   
   Not the tricky case:
   
   * If the `DICT` does not have a key, then we may want a `hasKey()` method, and the value reader should be a Java `null` because it does not exist.
   
   Or:
   
   * If the `DICT` does not have a key, then the value reader's `isNull()` returns true. (This means we cannot differentiate the has-key-with-null-value vs. does-not-have-a-key cases.)
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services