Satheesh Bandaram wrote:
I am submitting this patch for a VOTE. It has been pending for about a week. My vote is "+1", with the following comments. Since this is a new feature, I think, three +1 votes are requied. Here is the status of this patch. I am basically waiting for the final +1 vote....Satheesh, I think that you are right on both points. IntersectOrExceptNode should refer to SetOpResultSet instead of SetOpProjectRestrict.
1. It passed build and all tests.
2. Mike and myself have voted +1.
3. Dan provided a suggestion, with some syntax improvement. Any response from the contributor? I am assuming Dan's vote is a +1. If not, please speak up.. :-)
Here are my comments:
1. IntersectOrExceptNode still refers to SetOpProjectRestrict. Should this be SetOpResultSet?
2. Doesn't tableConstructor logic apply only to UnionNode? If so, should the fields like tableConstructor, topTableConstructor and methods like setTableConstructorTypes() be moved to UnionNode? Current code in SetOperatorNode refers to subclass UnionNode a lot, which could be improved?
From reading the code I gather that the tableConstructor field is used to mark a Union node that is generated from a VALUES expression that has more than one row. So, while the expression (t1 INTERSECT t2) sonstructs a table, it will never be a "tableConstructor" in the narrower sense used by our code.
I will change the code accordingly and submit a new patch. It should be ready today or first thing tomorrow morning.
Jack
