paul-rogers commented on a change in pull request #1945: DRILL-7502: Invalid
codegen for typeof() with UNION
URL: https://github.com/apache/drill/pull/1945#discussion_r363012403
##########
File path: common/src/main/java/org/apache/drill/common/types/Types.java
##########
@@ -906,4 +908,16 @@ public static boolean isNullable(final MajorType type) {
throw new UnsupportedOperationException("Unexpected/unhandled DataMode
value " + type.getMode());
}
}
+
+ /**
+ * The number of minor types. Actually, the largest minor type ordinal.
+ * (There are holes in the ordering.) Update this if a new type
+ * is added. Use this value when allocating arrays to be indexed
+ * by minor type.
+ *
+ * @return the maximum minor type ordinal
+ */
+ public static final int typeCount() {
Review comment:
@vvysotskyi, if I could e-mail you a beer, or even a pizza, I would do so.
You uncovered a huge mess in the Union vector and reader. It is amazing it ever
worked. Adding the `DICT` type pushed it over the edge. As it turns out, the
code has long confused ordinals and Protobuf values. The problem is made worse
because the Probuf-generated classes replace the `valueOf()` ordinal function
with a lookup based on the Protobuf values. We had the two systems all mixed up.
So, I cleaned up all that, following your advice to use only the ordinal
values. Added comments to help people avoid falling into this trap in the
future.
Plus, I think this accidentally fixed another issue: DRILL-7510.
Not bad for a single review comment!
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
