asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Heri Ramampiaro (Code Review)" <>
Subject Change in asterixdb[master]: ASTERIXDB-1187 and ASTERIXDB-1162 fixes, as well as updates ...
Date Sat, 19 Dec 2015 09:42:41 GMT
Heri Ramampiaro has posted comments on this change.

Change subject: ASTERIXDB-1187 and ASTERIXDB-1162 fixes, as well as updates on the internal

Patch Set 16:


Thank you for the comments suggestion. I am trying to make the changes. 

Btw. I think the only difference is on the default order field/variable listing, but not the
format itself. I am not sure if these are specified in the XML-file, as I am using the same
XML file (unless that has been updated...). Do you think this would matter? 

File asterix-om/src/main/java/org/apache/asterix/builders/

Line 56:     private final ByteArrayAccessibleOutputStream openPartOutputStream;
> These fields can be moved to specific functions which need to dynamically c
These fields above were actually used in the original version of the RecordBuilder. The following
are those, I added. In any case, I can move the dynamic check to functions that need to avoid
adding closed record in open fields... (which could currently (theoretically) be a runtime

Line 187:         if (arg.first == true)
> The dynamic check in line 182 to line 188 seems only being used by record-m
File asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/

Line 45:     public static IAType mergedNestedType(IAType fieldType1, IAType fieldType0) throws
> name those parameters properly, e.g.:

Line 48:             throw new AlgebricksException("Duplicate field " + fieldType1.getTypeName()
+ " encountered");
> The exception message seems not accurate enough.
File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/

Line 66:                         return true;
> wrap a single line if-else-block with {}.

Line 74:                 return false;
> wrap a single line if-else-block with {}.
File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/

Line 49:     private final AMutableString aString = new AMutableString("");
> The same comments can be found in the other code review:
File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/

Line 60:     private final Deque<IVisitablePointable> recordPath = new ArrayDeque<>();
> Potential race conditions: recordPath is shared by multiple partitions.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I3621ebdd71c7cd91b50d77a972ad863cea7fcbc2
Gerrit-PatchSet: 16
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Heri Ramampiaro <>
Gerrit-Reviewer: Heri Ramampiaro <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Preston Carman <>
Gerrit-Reviewer: Steven Jacobs <>
Gerrit-Reviewer: Till Westmann <>
Gerrit-Reviewer: Yingyi Bu <>
Gerrit-HasComments: Yes

View raw message