Details
-
Type:
Sub-task
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: 1.11.0
-
Component/s: None
-
Labels:None
Description
Interesect distinct will be rewritten as
Rewrite: (GB-Union All-GB)-GB-FIL-Proj
Example: R1 Intersect All R2
R3 = GB(R1 on all attributes + count() as c) union all GB(R2 on all attributes + count() as c)
R4 = GB(R3 on all attributes + count(c) as cnt)
R5 = Fil ( cnt == #branch )
R6 = Proj(R5 on all attributes)
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Julian Hyde, Jesus Camacho Rodriguez and Ashutosh Chauhan could u take a look? Thanks.
Show
Pengcheng Xiong
added a comment - Julian Hyde , Jesus Camacho Rodriguez and Ashutosh Chauhan could u take a look? Thanks.
Can you make a PR please?
Show
Julian Hyde
added a comment - Can you make a PR please?
Show
Pengcheng Xiong
added a comment - https://github.com/apache/calcite/pull/313
Thanks; I'll take a look tomorrow.
Show
Julian Hyde
added a comment - Thanks; I'll take a look tomorrow.
- Please add necessary markup around javadoc so that it stays formatted when converted to html
- Remove the deprecated constructor - this is a new class
- Add a @see in UnionToDistinctRule
- Can you inline your calls to makeBigintLiteral. (We ought to have deprecated that method, but let's not start using it.)
- I think you should change count(1) to count(*). It's the same (because 1 is not null) but doesn't entail a project. Then, I don't think the lower Aggregate calls need a count at all.
- Make relBuilder final. It's safe to re-use the RelBuilder.
- Sorry to be pedantic, but can you rename numBranches to inputCount? Consistent with terminology and naming elsewhere.
- Can you enable JdbcFrontLinqBackTest.testIntersect. With your changes, I think it should pass!
Show
Julian Hyde
added a comment - - edited
Please add necessary markup around javadoc so that it stays formatted when converted to html
Remove the deprecated constructor - this is a new class
Add a @see in UnionToDistinctRule
Can you inline your calls to makeBigintLiteral . (We ought to have deprecated that method, but let's not start using it.)
I think you should change count(1) to count(*) . It's the same (because 1 is not null) but doesn't entail a project. Then, I don't think the lower Aggregate calls need a count at all.
Make relBuilder final. It's safe to re-use the RelBuilder.
Sorry to be pedantic, but can you rename numBranches to inputCount ? Consistent with terminology and naming elsewhere.
Can you enable JdbcFrontLinqBackTest.testIntersect . With your changes, I think it should pass!
sure，i will follow your comments.
Show
Pengcheng Xiong
added a comment - sure，i will follow your comments.
Show
Pengcheng Xiong
added a comment - new PR: https://github.com/apache/calcite/pull/323
Looking good... I just need to do some final testing before I push to master.
Show
Julian Hyde
added a comment - Looking good... I just need to do some final testing before I push to master.
Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/76fef36c. Pengcheng Xiong, thanks for the PR!
Show
Julian Hyde
added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/76fef36c . Pengcheng Xiong , thanks for the PR!
Resolved in release 1.11.0 (2017-01-11).
Show
Julian Hyde
added a comment - Resolved in release 1.11.0 (2017-01-11).
I believe that Calcite already supports INTERSECT DISTINCT. At least in the parser.