subject:"Change in asterixdb\[master\]\: \[COMP\] Maintain schemas for delegate operators"

Change in asterixdb[master]: [COMP] Maintain schemas for delegate operators

2018-06-27 Thread Steven Jacobs (Code Review)

Steven Jacobs has abandoned this change.

Change subject: [COMP] Maintain schemas for delegate operators
..


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Icc8723e686d741bf3098ff44d046cde5239e204b
Gerrit-PatchSet: 7
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Steven Jacobs 
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin 
Gerrit-Reviewer: Jenkins 
Gerrit-Reviewer: Steven Jacobs 
Gerrit-Reviewer: Xikui Wang

Change in asterixdb[master]: [COMP] Maintain schemas for delegate operators

2018-06-27 Thread Steven Jacobs (Code Review)

Steven Jacobs has posted comments on this change.

Change subject: [COMP] Maintain schemas for delegate operators
..

Patch Set 7:

(1 comment)

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

Line 93: return delegate.getSchema();
> I still don't see why this is needed. DelegateOperator is the owner of the
The issue here is that getSchema() and setSchema() don't currently match. One
sets the delegate schema only and one gets the DelegateOperator schema only.
This will not work properly when the schema is re-calculated bottom-up for an
operator above the DelegateOperator. setSchema() will set the schema for the
delegate, and then when an operator above calls getSchema(), it won't be the
one that was set.

There is currently no situation where it comes up, but I saw it when I was
messing with plans on the BAD branch. I think the change is needed to make it
future-proof. There are two choices:

1) Keep the schema on the DelegateOperator
2) Keep the schema on the delegate

I think both will work fine. I originally did (2) but Xikui suggested that (1)
was better. I don't care either way, but the main thing is that getSchema() and
setSchema() need to match in some way. Let me know what you think.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc8723e686d741bf3098ff44d046cde5239e204b
Gerrit-PatchSet: 7
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Steven Jacobs
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin
Gerrit-Reviewer: Jenkins
Gerrit-Reviewer: Steven Jacobs
Gerrit-Reviewer: Xikui Wang
Gerrit-HasComments: Yes

Change in asterixdb[master]: [COMP] Maintain schemas for delegate operators

2018-06-27 Thread Dmitry Lychagin (Code Review)

Dmitry Lychagin has posted comments on this change.

Change subject: [COMP] Maintain schemas for delegate operators
..

Patch Set 7:

(1 comment)

Line 93: return delegate.getSchema();
I still don't see why this is needed. DelegateOperator is the owner of the
schema because it can recompute it from its inputs (recomputeSchema()). Every
time it does that it makes the schema available to IOperatorDelegate
(delegate.setSchema()). The delegate cannot compute the schema by itself, so
cannot be its owner. It looks like DelegateOperator.schema and
AbstractDelegatedLogicalOperator.schema should always point to the same list of
variables. Is it not the case? When can they diverge? Do you have an example
describing the problem or an existing issue that was filed?

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

Change in asterixdb[master]: [COMP] Maintain schemas for delegate operators

2018-06-26 Thread Anon. E. Moose (Code Review)

Anon. E. Moose #1000171 has posted comments on this change.

Change subject: [COMP] Maintain schemas for delegate operators
..


Patch Set 7: Contrib+1

Analytics Compatibility Tests Successful
https://goo.gl/u4mZgP : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc8723e686d741bf3098ff44d046cde5239e204b
Gerrit-PatchSet: 7
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Steven Jacobs 
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin 
Gerrit-Reviewer: Jenkins 
Gerrit-Reviewer: Steven Jacobs 
Gerrit-Reviewer: Xikui Wang 
Gerrit-HasComments: No

1 2 >

1 - 100 of 141 matches

Mail list logo