asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Heri Ramampiaro (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
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
functions
......................................................................


Patch Set 16:

(8 comments)

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? 

Thanks,
-heri

https://asterix-gerrit.ics.uci.edu/#/c/298/16/asterix-om/src/main/java/org/apache/asterix/builders/RecordBuilder.java
File asterix-om/src/main/java/org/apache/asterix/builders/RecordBuilder.java:

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
issue).


Line 187:         if (arg.first == true)
> The dynamic check in line 182 to line 188 seems only being used by record-m
Done


https://asterix-gerrit.ics.uci.edu/#/c/298/16/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/AbstractRecordManipulationTypeComputer.java
File asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/AbstractRecordManipulationTypeComputer.java:

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


Line 48:             throw new AlgebricksException("Duplicate field " + fieldType1.getTypeName()
+ " encountered");
> The exception message seems not accurate enough.
Done


https://asterix-gerrit.ics.uci.edu/#/c/298/16/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/DeepEqualAssessor.java
File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/DeepEqualAssessor.java:

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


Line 74:                 return false;
> wrap a single line if-else-block with {}.
Done


https://asterix-gerrit.ics.uci.edu/#/c/298/16/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableUtils.java
File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableUtils.java:

Line 49:     private final AMutableString aString = new AMutableString("");
> The same comments can be found in the other code review:
Done


https://asterix-gerrit.ics.uci.edu/#/c/298/16/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordRemoveFieldsEvalFactory.java
File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordRemoveFieldsEvalFactory.java:

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


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3621ebdd71c7cd91b50d77a972ad863cea7fcbc2
Gerrit-PatchSet: 16
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Heri Ramampiaro <heriram@gmail.com>
Gerrit-Reviewer: Heri Ramampiaro <heriram@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Preston Carman <prestonc@apache.org>
Gerrit-Reviewer: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message