asterixdb-notifications mailing list archives

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

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

Patch Set 16:


Detailed comments are inlined.
BTW, it seems the CodeStyle is still different from what I have. If I format your code in
my Eclipse, I still can see the files are changed...
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 check duplicate

Line 187:         if (arg.first == true)
The dynamic check in line 182 to line 188 seems only being used by record-merge function and
record-add-field function because the compiler couldn't statically check that.

RecordBuilders are used in almost every query and might be called for *every* tuple, e.g.,
in the return clause.  For those usual cases, the check already happens in the compiler, hence
this runtime check is not needed and will add some overheads.
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.:
mergeNestedType --> mergeNestedRecordType

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: Ian Maxon <>
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