asterixdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ildar Absalyamov (Code Review)" <do-not-re...@asterix-gerrit.ics.uci.edu>
Subject Change in asterixdb[master]: Changed metadata storage format for nullable field types. Mo...
Date Fri, 17 Jul 2015 07:48:45 GMT
Ildar Absalyamov 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)

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?
Done


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?
We do keep AUnionType since it exists in the runtime. However the enum DerivedTypeTag, which
is used in this switch statement is used to describe only the derived type, which are persisted
in metadata, hence union is removed.


Line 378:         } catch (Exception e) {
> Could you elaborate a little more on this TODO?
Well this comment is inherited from the old writeRecordType method. The exception model is
totally messed up, and not only here. I've tried to resolve it but 1 change lead to whole
bunch of others, I am not sure it's in the context of that change


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?
It seems that the only types, which are allowed here are NULL or INT64?
So this code should not be reached anyway. The previous version of the code would have returned
union without nullable type, which is not a valid type. I believe explicitly throwing exception
in this situation (which is not technically possible) is better then returning not valid of
null type


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
ditto


-- 
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 <ildar.absalyamov@gmail.com>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Ildar Absalyamov <ildar.absalyamov@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Carey <dtabass@gmail.com>
Gerrit-Reviewer: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-HasComments: Yes

Mime
View raw message