asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yingyi Bu (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Supports Left Outer Join and Left Outer Unnest in SQL++.
Date Sat, 04 Jun 2016 01:20:45 GMT
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 <buyingyi@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message