You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@asterixdb.apache.org by "Ian Maxon (Code Review)" <ge...@unhygienix.ics.uci.edu> on 2015/07/17 01:15:43 UTC

Change in asterixdb[master]: Changed metadata storage format for nullable field types. Mo...

Ian Maxon has posted comments on this change.

Change subject: Changed metadata storage format for nullable field types. Moved field name generation to the client out of metadata node code. Changed naming scheme for autogenerated types.
......................................................................


Patch Set 1:

(5 comments)

Just a few comments. Very cool otherwise! Much cleaner than the weird on the fly construction of UNION(NULL, fooType) wherever one needed to decide if something was optional or not.

https://asterix-gerrit.ics.uci.edu/#/c/323/1/asterix-metadata/src/main/java/edu/uci/ics/asterix/metadata/bootstrap/MetadataPrimaryIndexes.java
File asterix-metadata/src/main/java/edu/uci/ics/asterix/metadata/bootstrap/MetadataPrimaryIndexes.java:

Line 120:         //                FEED_ACTIVITY_DATASET_ID, true, new int[] { 0, 1, 2, 3 });
Is this block supposed to be commented out, still?


https://asterix-gerrit.ics.uci.edu/#/c/323/1/asterix-metadata/src/main/java/edu/uci/ics/asterix/metadata/entitytupletranslators/DatatypeTupleTranslator.java
File asterix-metadata/src/main/java/edu/uci/ics/asterix/metadata/entitytupletranslators/DatatypeTupleTranslator.java:

Line 148:                 }
Do we actually want to totally remove UNION type?


Line 378:         } catch (Exception e) {
Could you elaborate a little more on this TODO?


https://asterix-gerrit.ics.uci.edu/#/c/323/1/asterix-om/src/main/java/edu/uci/ics/asterix/om/typecomputer/impl/UnaryBinaryInt64OrNullTypeComputer.java
File asterix-om/src/main/java/edu/uci/ics/asterix/om/typecomputer/impl/UnaryBinaryInt64OrNullTypeComputer.java:

Line 57: 
What's the reason here for the exception thrown rather than return?


https://asterix-gerrit.ics.uci.edu/#/c/323/1/asterix-om/src/main/java/edu/uci/ics/asterix/om/typecomputer/impl/UnaryStringInt64OrNullTypeComputer.java
File asterix-om/src/main/java/edu/uci/ics/asterix/om/typecomputer/impl/UnaryStringInt64OrNullTypeComputer.java:

Line 71: 
Same question r.e. exception rather than return


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/323
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I223aded8aaf80f0688358899c0e8b0d6988fac93
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ildar Absalyamov <il...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Steven Jacobs <sj...@ucr.edu>
Gerrit-HasComments: Yes