Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: support using clause in join statement. #3372

Merged
merged 6 commits into from Jun 12, 2017

Conversation

@bobotu
Copy link
Member

commented Jun 1, 2017

Try to support using in join.

@shenli

This comment has been minimized.

Copy link
Member

commented Jun 1, 2017

@bobotu Thanks!
@hanfei1991 PTAL

)

if lc, err = lsc.FindColumn(col); err != nil {
return err

This comment has been minimized.

Copy link
@XuHuaiyu

XuHuaiyu Jun 2, 2017

Contributor

errors.Trace(err)
same as line 295, 303

p.EqualConditions = append(conds, p.EqualConditions...)

coalescedSchema := expression.NewSchema(coalescedCols...)
if left, ok := leftPlan.(*LogicalJoin); ok && left.coalescedSchema != nil {

This comment has been minimized.

Copy link
@XuHuaiyu

XuHuaiyu Jun 2, 2017

Contributor

What does this for,
why not check rightPlan?

This comment has been minimized.

Copy link
@bobotu

bobotu Jun 2, 2017

Author Member

To support something like this

SELECT t1.a, t2.a, t3.a FROM t1 JOIN t2 USING (a) JOIN t3 USING (a);
@hanfei1991

This comment has been minimized.

Copy link
Member

commented Jun 2, 2017

if a join b using (c1) join c using(c1) and a.c1 is int, b.c1 is string, the join c will use which one ?

@hanfei1991

This comment has been minimized.

Copy link
Member

commented Jun 2, 2017

I think it will introduce too much complexity to support using clause. What do you think ? @shenli

@bobotu

This comment has been minimized.

Copy link
Member Author

commented Jun 2, 2017

Now, I just simply follow these rules:

If a column in the USING clause is referenced without being qualified by a table name, the column reference points to the column in the first (left) table if the join is an INNER JOIN or a LEFT OUTER JOIN. If it is a RIGHT OUTER JOIN, unqualified references to a column in the USING clause point to the column in the second (right) table.

So, join c will use a.c1. @hanfei1991

if join, ok := er.p.(*LogicalJoin); ok && join.coalescedSchema != nil {
column, err := join.coalescedSchema.FindColumn(v)
if err != nil {
er.err = ErrAmbiguous.GenByArgs(v.Name)

This comment has been minimized.

Copy link
@coocood

coocood Jun 9, 2017

Member

Why not just errors.Trace(err)?
And it's impossible for coalesced schema to return an ambiguous error.

if !coalesced[col.ColName.L] {
schemaCols = append(schemaCols, col)
}
}

This comment has been minimized.

Copy link
@coocood

coocood Jun 9, 2017

Member

Just schemaCols = append(schemaCols, lsc.Columns) will do.

This comment has been minimized.

Copy link
@bobotu

bobotu Jun 10, 2017

Author Member

According to MySQL

Redundant column elimination and column ordering occurs according to standard SQL, producing this display order:

First, coalesced common columns of the two joined tables, in the order in which they occur in the first table

Second, columns unique to the first table, in order in which they occur in that table

Third, columns unique to the second table, in order in which they occur in that table

And the first table is the right hand side table in right join

So I first found the columns which in both table, then sort them based on their position in source table, then switch the lsc and rsc if there is a right join, finally I append remain columns in lsc and rsc.


p.SetSchema(expression.NewSchema(schemaCols...))
p.EqualConditions = append(conds, p.EqualConditions...)
p.coalescedSchema = expression.MergeSchema(p.coalescedSchema, expression.NewSchema(coalescedCols...))

This comment has been minimized.

Copy link
@coocood

coocood Jun 9, 2017

Member

Isn't the coalescedSchema empty, why do we need to merge it?

This comment has been minimized.

Copy link
@bobotu

bobotu Jun 9, 2017

Author Member

coalescedSchema is not empty in

select * from (t1 join t2 using (a)) join (t3 join t4 using (a)) on (t2.a = t4.a and t1.a = t3.a)

L231-L238 merge the sub join's coalescedSchema.

This comment has been minimized.

Copy link
@coocood

coocood Jun 9, 2017

Member

Why do we need to copy sub join's coalescedSchema to outer join?
For example.

select * from (t1 join t2 using (a)) join t3 using (b)

The outer join's coalesedSchema should be b rather than a, b.

If t3 doesn't have column a.

select * from (t1 join t2 using (a)) join t3 using (a)

should return error.

This comment has been minimized.

Copy link
@bobotu

bobotu Jun 9, 2017

Author Member

But I see we can access coalesced columns explicit in MySQL:

select t2.a from (t1 join t2 using (a)) join t3 using (a);
select * from (t1 join t2 using (a) join t4 using (a)) join t3 on (t1.a = t2.a);

t2.a is coalesced in t1 join t2 using (a), but we can access it in outer join. So if we don't merge the coalescedSchema into outer join, we must search all sub join's coalescedSchema in toColumn.

This comment has been minimized.

Copy link
@coocood

coocood Jun 10, 2017

Member

@bobotu
Then just search all sub join's coalescesSchema if we can't find it in normal join schema?
I think this is simpler and clearer.

This comment has been minimized.

Copy link
@coocood

coocood Jun 10, 2017

Member

@bobotu
OK, I think I misunderstood coalescedSchema, it's the column will not be used.
The name is misleading.
For example, a and b coalesced into a, a should be the coalesced column instead of b.
And the comment for it should be more specific.

This comment has been minimized.

Copy link
@bobotu

bobotu Jun 10, 2017

Author Member

Sorry, I'll change it's name and specify the comment.

@coocood

This comment has been minimized.

Copy link
Member

commented Jun 12, 2017

LGTM

@bobotu
Well done!

@coocood coocood added the status/LGT1 label Jun 12, 2017

@coocood

This comment has been minimized.

Copy link
Member

commented Jun 12, 2017

@shenli
shenli approved these changes Jun 12, 2017
Copy link
Member

left a comment

LGTM

@shenli shenli merged commit 4bc3cf7 into pingcap:master Jun 12, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@bobotu bobotu deleted the bobotu:using-clause branch Jun 12, 2017

@bobotu bobotu referenced this pull request Jul 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.