abdullah alamoudi has posted comments on this change. Change subject: Support Change Feeds and Ingestion of Records with MetaData ......................................................................
Patch Set 5: (19 comments) https://asterix-gerrit.ics.uci.edu/#/c/620/5/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/metadata/IMetadataProvider.java File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/metadata/IMetadataProvider.java: Line 73: List<Mutable<ILogicalExpression>> additionalNonFilteringKeys) throws AlgebricksException; > Could we move this list of variables next to the other lists of variables i Done Line 73: List<Mutable<ILogicalExpression>> additionalNonFilteringKeys) throws AlgebricksException; > Can we keep this interface unchanged at all? That would be very difficult to do. since this is called from Hyracks in InsertDeleteUpsertPOperator. In order to do this without changing this interface at all would be really difficult. Line 75: public default Pair<IOperatorDescriptor, AlgebricksPartitionConstraint> getInsertRuntime( > method is not used Done Line 212: List<Mutable<ILogicalExpression>> additionalNonFilteringKeys) throws AlgebricksException; > Could we move this list of variables next to the other lists of variables i Done Line 214: public default Pair<IOperatorDescriptor, AlgebricksPartitionConstraint> getUpsertRuntime( > method is not used Done https://asterix-gerrit.ics.uci.edu/#/c/620/5/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/ProjectOperator.java File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/ProjectOperator.java: Line 42: private final List<Mutable<ILogicalExpression>> expressions; > What is this list of expressions used for? This list will store expressions that will become variables at some point. because otherwise, new variables introduced will not propagate. Line 60: this(v, new ArrayList<Mutable<ILogicalExpression>>()); > Why do we add an empty list here but not in the other constructor? because this is the constructor used in the translator. Hence, we need the list. In the optimizer, if we don't need the list, we better not create one. Line 119: public List<Mutable<ILogicalExpression>> getExpressions() { > Could we remove this method and just pass the correct set of expressions to That is difficult to do since this object is created at the translator level and we wouldn't know if we need expressions at all at that time. https://asterix-gerrit.ics.uci.edu/#/c/620/5/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/UnnestOperator.java File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/UnnestOperator.java: Line 73: target.addAllVariables(sources[0]); > what's the difference with the one in its super class? When I undid the previous patch, I didn't notice this. Done https://asterix-gerrit.ics.uci.edu/#/c/620/5/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/plan/ALogicalPlanImpl.java File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/plan/ALogicalPlanImpl.java: Line 56: public static String prittyPrintPlan(ILogicalPlan plan) throws AlgebricksException { > s/pritty/pretty/ Done https://asterix-gerrit.ics.uci.edu/#/c/620/5/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/LogicalOperatorPrettyPrintVisitor.java File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/LogicalOperatorPrettyPrintVisitor.java: Line 410: pprintExprList(op.getPrevSecondaryKeyExprs(), buffer, indent); > Is this the same line twice? huh. how did that get here!! https://asterix-gerrit.ics.uci.edu/#/c/620/5/algebricks/algebricks-examples/piglet-example/src/main/java/org/apache/hyracks/algebricks/examples/piglet/metadata/PigletMetadataProvider.java File algebricks/algebricks-examples/piglet-example/src/main/java/org/apache/hyracks/algebricks/examples/piglet/metadata/PigletMetadataProvider.java: Line 245: public Pair<IOperatorDescriptor, AlgebricksPartitionConstraint> getInsertRuntime( > Could we put these back into the order they were in before? Done https://asterix-gerrit.ics.uci.edu/#/c/620/5/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java File algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java: Line 104: ignoreOps.add(LogicalOperatorTag.INSERT_DELETE_UPSERT); > Is this the only change in behavior in this file? Done Line 273: if ((expr.getExpressionTag() != LogicalExpressionTag.VARIABLE) > Why do we need these additional parentheses? It seems that the precedence i Again, this is another Eclipse auto edit option. I am disabling it. https://asterix-gerrit.ics.uci.edu/#/c/620/5/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/comm/io/ArrayTupleBuilder.java File hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/comm/io/ArrayTupleBuilder.java: Line 37: @Deprecated > please add "use IFrameFieldAppender.appendField to append fields directly" Done https://asterix-gerrit.ics.uci.edu/#/c/620/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/file/FieldCursorForDelimitedDataParser.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/file/FieldCursorForDelimitedDataParser.java: Line 46: private final Reader in; //the underlying buffer > Is there an actual change in this file? Done https://asterix-gerrit.ics.uci.edu/#/c/620/5/hyracks/hyracks-dataflow-std/src/test/java/org/apache/hyracks/dataflow/std/test/file/CursorTest.java File hyracks/hyracks-dataflow-std/src/test/java/org/apache/hyracks/dataflow/std/test/file/CursorTest.java: Line 19: package org.apache.hyracks.dataflow.std.test.file; > Can we keep this in package "org.apache.hyracks.dataflow.std.file"? Done Line 77: if (cursor.isDoubleQuoteIncludedInThisField) { > Why is this done in the test? Shouldn't the cursor do that? For some reason, it doesn't. This needs to be called specifically. Line 84: System.err.println("Test case failed. Expected nuumber of fields in each record is " + numOfFields > s/nuumber/number/ Done -- To view, visit https://asterix-gerrit.ics.uci.edu/620 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3749349e2b9f1b03c8b310eb99d3f44d08be77df Gerrit-PatchSet: 5 Gerrit-Project: hyracks Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Ildar Absalyamov <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
