Yingyi Bu has posted comments on this change. Change subject: Supports Left Outer Join and Left Outer Unnest in SQL++. ......................................................................
Patch Set 12: (16 comments) https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ByNameToByIndexFieldAccessRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ByNameToByIndexFieldAccessRule.java: Line 99: IOptimizationContext context) throws AlgebricksException { > Remove template params in this method? Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java: Line 246: * @param toPush > update comment? Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java: Line 425: new MutableObject<ILogicalExpression>(makeUnnestExpression(eo.first)), pVar, > remove the template parameter? Done Line 481: throw new IllegalStateException(ERR_MSG); > Should this just be an UnsupportedOperationException? Do we have a "usual" Done Line 487: throw new IllegalStateException(ERR_MSG); > Should this just be an UnsupportedOperationException? Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectUtil.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectUtil.java: Line 329: boolean hasNullableFields = NonTaggedFormatUtil.hasOptionalField(recordType); > rename this variable? Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/ClassAdParser.java File asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/ClassAdParser.java: Line 358: "Field: " + recType.getFieldNames()[optionalFieldId] + " can not be null"); > s/null/optional/? Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/serde/ARecordSerializerDeserializer.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/serde/ARecordSerializerDeserializer.java: Line 112: boolean hasNullableFields = NonTaggedFormatUtil.hasOptionalField(this.recordType); > rename variable? Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/ARecordVisitablePointable.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/ARecordVisitablePointable.java: Line 191: boolean hasNullableFields = NonTaggedFormatUtil.hasOptionalField(inputRecType); > rename variable? Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/cast/ARecordCaster.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/cast/ARecordCaster.java: Line 111: missingTypeTag.set(bos.getByteArray(), start, end - start); > What's the difference between missingReference and missingTypeTag? Done Line 142: } catch (AsterixException e) { > This seems to be the only place where we use TypeException. Could we remove Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/nonvisitor/ARecordPointable.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/nonvisitor/ARecordPointable.java: Line 159: > s/getExpendedOffset/getExpandedOffset/ Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/printer/ARecordPrinter.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/printer/ARecordPrinter.java: Line 81: .deserialize(nextFieldValue.getByteArray()[nextFieldValue.getStartOffset()]); > Would it make sense to re-use this typeTag for the next iteration (and get Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/RecordRemoveFieldsTypeComputer.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/RecordRemoveFieldsTypeComputer.java: Line 337: } else { > Should we just remove this else and fall through to the default case (addin Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessByIndexEvalFactory.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/FieldAccessByIndexEvalFactory.java: Line 94: eval1.evaluate(tuple, inputArg1); > Should we check the type of the 2nd argument here? Done https://asterix-gerrit.ics.uci.edu/#/c/899/12/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/IsomorphismVariableMappingVisitor.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/IsomorphismVariableMappingVisitor.java: Line 473: if (variableMapping.get(right).equals(left)) { > Just return Done -- To view, visit https://asterix-gerrit.ics.uci.edu/899 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie0caea9c1842d93541b067a1193d117af30d8dfc Gerrit-PatchSet: 12 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-HasComments: Yes
