asterixdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Preston Carman (Code Review)" <>
Subject Change in asterixdb[master]: Clean-up - Made record-merge visible from AQL + more proper ...
Date Mon, 20 Jul 2015 21:20:20 GMT
Preston Carman has posted comments on this change.

Change subject: Clean-up - Made record-merge visible from AQL + more proper dealing with nested
records in record merge type computer

Patch Set 7:


Here are a few of my thoughts.
File asterix-doc/src/site/markdown/aql/

Line 375: ### uppercase ###
Is uppercase listed twice in our documentation?

Line 2232: 
Can you add a better/more explaination on the idea of open status?

Line 2309:           "address":{"state":"CA"}, 
Does the function change the order of fields? If so, note that otherwise update the result.

Line 2548:  * Arguments:
Do think we should make some notes here about what equality means? Based on our friday meeting,
I wonder if we need to add some detail to this explanation.
File asterix-metadata/src/main/java/edu/uci/ics/asterix/metadata/bootstrap/

Line 19
Has this file been changed? If not, maybe remove this change.
File asterix-om/src/main/java/edu/uci/ics/asterix/om/typecomputer/impl/

Line 49:     private final Deque<List<IAType>> aTypelistPool = new ArrayDeque<>();
What about using a ListObjectPool object here?
File asterix-om/src/main/java/edu/uci/ics/asterix/om/typecomputer/impl/

Line 134
Is this used in other places? Why move this to abstract class?
File asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/evaluators/comparisons/

Line 54:         if (PointableUtils.getTypeTag(vp0) != PointableUtils.getTypeTag(vp1))
Add a note about only checking matching types, not equivalence.
File asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/evaluators/functions/

Line 64:             private ISerializerDeserializer boolSerde = AqlSerializerDeserializerProvider.INSTANCE
can this be final?

Line 84:                             Pair<IVisitablePointable, Boolean>(accessor1, Boolean.FALSE);
Why Boolean over boolean?
File asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/evaluators/functions/

Line 160:     public static UTF8StringPointable serializeString(String str, IMutableValueStorage
vs) throws AlgebricksException {
Would this function be better to pass the returned pointable as a parameter? This would allow
for object allocation to be defined by calling functions. Allowing reusing of the UTF8StringPointable.
File asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/evaluators/functions/records/

Line 187:                             IVisitablePointable valuePointable = null;
Can these variable be moved up and reused?

Line 192:                                 IVisitablePointable fieldName = names.get(j);
Can this variable be moved up and resulted?
File asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/evaluators/visitors/

Line 39:     private final int TABLE_SIZE = 100;
In our meeting on Friday they talked about a way to do this dynamically. Please look into

Line 40:     private final int TABLE_FRAME_SIZE = 32768;
Can you use the Hyracks variable for frame size?

Line 104:             keyEntry.set(buf, off, len);
In several places you make buf, off, len for the set function. Is there a reason to make them
variables as opposed to just passing them as arguments?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I3621ebdd71c7cd91b50d77a972ad863cea7fcbc2
Gerrit-PatchSet: 7
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Heri Ramampiaro <>
Gerrit-Reviewer: Heri Ramampiaro <>
Gerrit-Reviewer: Ian Maxon <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Preston Carman <>
Gerrit-Reviewer: Steven Jacobs <>
Gerrit-HasComments: Yes

View raw message