Yingyi Bu has posted comments on this change.

Change subject: Support Change Feeds and Ingestion of Records with MetaData
......................................................................


Patch Set 8:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/620/8/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 72:             List<Mutable<ILogicalExpression>> 
additionalNonFilteringKeys, RecordDescriptor recordDesc,
List<Mutable<ILogicalExpression>> additionalNonFilteringKeys --> 
List<LogicalVariable> additionalNonFilteringKeys

Here we always talk about variables, e.g., keys, payloadVar, 
additionalFilterKeyFields.


Line 200:             LogicalVariable payLoadVar, List<LogicalVariable> 
additionalFilterFields,
List<Mutable<ILogicalExpression>> additionalNonFilteringKeys --> 
List<LogicalVariable> additionalNonFilteringKeys


https://asterix-gerrit.ics.uci.edu/#/c/620/8/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/InsertDeleteUpsertOperator.java
File 
algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/InsertDeleteUpsertOperator.java:

Line 177:     public void setAdditionalNonFilteringExpressions(
remove this set method?  It's better to make things immutable, which is easier 
for human code analysis.


https://asterix-gerrit.ics.uci.edu/#/c/620/8/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 51:         this.getVariables().add(v);
>>This list will store expressions that will become variables at some 
>>point. because otherwise, new variables introduced will not propagate.

I still couldn't understand that.  In principle, a project operator should only 
work with a set of variables.  I guess the expressions here are lineages of 
those variables.  If so, this seems not right because (1) it seems to add 
redundant info into the project operator because we can always analyze the plan 
to get what the up-to-date expressions are, (2) variable definitions can be 
changed by other rewriting rules and thus the variable list and expression list 
in this class could become out-of-date.


-- 
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: 8
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

Reply via email to