Till Westmann has posted comments on this change. Change subject: ASTERIXDB-1187 and ASTERIXDB-1162 fixes, as well as updates on the internal functions ......................................................................
Patch Set 17: (14 comments) Just some code hygiene :) https://asterix-gerrit.ics.uci.edu/#/c/298/17/asterix-om/src/main/java/org/apache/asterix/builders/CheckCompletelyOpenRecordTypeVisitor.java File asterix-om/src/main/java/org/apache/asterix/builders/CheckCompletelyOpenRecordTypeVisitor.java: Line 29: public class CheckCompletelyOpenRecordTypeVisitor implements IVisitablePointableVisitor<Void, Pair<Boolean, Void>> { The formatting of this file doesn't look right .. https://asterix-gerrit.ics.uci.edu/#/c/298/17/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 mergeNestedRecordType(IAType nestedRecordTypeLeft, IAType nestedRecordTypeRight) throws AlgebricksException, This method seems to be unused. Line 83: protected void addAdditionalFields(List<String> resultFieldNames, List<IAType> resultFieldTypes, This method seems to be unused. Line 111: protected List<String> getListFromExpression(ILogicalExpression expression) throws AlgebricksException { And this one seems to be only used in RecordRemoveFieldsTypeComputer. https://asterix-gerrit.ics.uci.edu/#/c/298/17/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/RecordAddFieldsTypeComputer.java File asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/RecordAddFieldsTypeComputer.java: Line 128: //additionalFieldNames.add(fieldName.getStringValue()); remove commented code? Line 136: //additionalFieldTypes.add(ft[j]); remove commented code? https://asterix-gerrit.ics.uci.edu/#/c/298/17/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/RecordRemoveFieldsTypeComputer.java File asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/RecordRemoveFieldsTypeComputer.java: Line 59: //private ARecordType inputRecordType; Can we remove this? https://asterix-gerrit.ics.uci.edu/#/c/298/17/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 67: if (leftVal == rightVal) { Just 'return leftVal == rightVal'? https://asterix-gerrit.ics.uci.edu/#/c/298/17/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java: Line 68: throws HyracksDataException { Does this need to throw HyracksDataException? Line 131: public void serializeString(String str, IMutableValueStorage vs) throws AsterixException { This method seems to be unused. https://asterix-gerrit.ics.uci.edu/#/c/298/17/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordRemoveFieldsDescriptor.java File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordRemoveFieldsDescriptor.java: Line 55: private AOrderedList pathAList; Is this used? Line 56: private List<List<String>> pathList; Is this used? https://asterix-gerrit.ics.uci.edu/#/c/298/17/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/visitors/ListDeepEqualityChecker.java File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/visitors/ListDeepEqualityChecker.java: Line 48: public ListDeepEqualityChecker(int tableSize, int tableFrameSize) { unused? Line 140: if (itemVisitorArg.second == false) curly braces -- 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: 17 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Heri Ramampiaro <[email protected]> Gerrit-Reviewer: Heri Ramampiaro <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Preston Carman <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-HasComments: Yes
