asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xikui Wang (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: [NO-ISSUE][COMP] Miscellaneous Fixes in Compiler
Date Thu, 09 Aug 2018 17:13:29 GMT
Xikui Wang has posted comments on this change.

Change subject: [NO-ISSUE][COMP] Miscellaneous Fixes in Compiler
......................................................................


Patch Set 1:

(3 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2858/1/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/constraints/AlgebricksAbsolutePartitionConstraint.java
File hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/constraints/AlgebricksAbsolutePartitionConstraint.java:

Line 31:         Arrays.sort(this.locations);
> Good catch. Can you extract it into a separate change? I'll approve it righ
This one is submitted.


https://asterix-gerrit.ics.uci.edu/#/c/2858/1/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AggregateOperator.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AggregateOperator.java:

Line 103:         IVariableTypeEnvironment env2 = this.computeInputTypeEnvironment(ctx);
> how is this different? aggregate operator only has one input, so the input'
I was having some type inference issue with this one, and I changed this. Sadly I didn't save
the query that I used. If you feel this is not necessary, we could skip this one.


https://asterix-gerrit.ics.uci.edu/#/c/2858/1/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceGroupByCombinerRule.java
File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceGroupByCombinerRule.java:

Line 91:                 newGbyLiveVars.add(usedDecorVars.get(0));
> do you have a testcase that was fixed by this? Can you include it into this
These bug fixes are separated from one of the other branches that I'm working on, thus I don't
have test cases for them. :( 
For this one, the problem that I had is there is a type inference issue in one of the group
query that I have. I found that the decoration variables are replicated multiple times due
to this. The replicated ones, the later copies, caused the issue. Since we are carrying those
variables anyway, I think copying duplicated ones are not necessary. The newly assigned ones
are not used anyway...
Btw, there is one more issue which i think was tricker so I saved the query. Here is the link:
https://issues.apache.org/jira/browse/ASTERIXDB-2436


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2858
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfc9459d0ccfc257e2c1b15f32d2697dcfbfcf42
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <xkkwww@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dmitry.lychagin@couchbase.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Xikui Wang <xkkwww@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message