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_r352252495
##########
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:
This approach is a bit better than the big null reader we had before. I
wonder, however, why we need such a reader? Null (AKA "dummy") readers exist,
but generally for specialized cases (such as a List that has no type.) I
wonder, is this for the case where the value of a key/value pair is null?
If so, then two thoughts. First, the right way to handle that is to:
* Use the `isNull()` method on the `ColumnReader` class:
```
if (myDictReader.value().isNull()) {
// Handle null value
} else {
int x = myDictReader.value().scalar().getInt();
}
```
The idea is that null values are null: they have no structure. The approach
here seems to say that null values have structure (such as a null union), so we
can ask what types the null has and so on. But, this is, perhaps, too much of a
fantasy.
The other suggestion would be to suggest that values cannot be null,
especially if they are of structured types.
Also, note that, for unions, we already have multiple levels of nulls: The
union itself can be null. When used in a list, the list entry can be null. The
union can have nullable types, so that the union might have an int, but the int
is null. This is already a mess, we probably don't want to make it even more
complex.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
