Skip to content

Commit

Permalink
*: Address comment
Browse files Browse the repository at this point in the history
  • Loading branch information
shenli committed Jan 14, 2016
1 parent 9a997f4 commit 7da9b26
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 9 deletions.
9 changes: 5 additions & 4 deletions optimizer/plan/planbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (b *planBuilder) extractSelectAgg(sel *ast.SelectStmt) []*ast.AggregateFunc
for _, item := range sel.OrderBy.Items {
n, ok := item.Expr.Accept(extractor)
if !ok {
b.err = errors.New("Failed to extract agg expr from orderbyu clause")
b.err = errors.New("Failed to extract agg expr from orderby clause")
return nil
}
item.Expr = n.(ast.ExprNode)
Expand All @@ -131,7 +131,8 @@ func (b *planBuilder) extractSelectAgg(sel *ast.SelectStmt) []*ast.AggregateFunc

func (b *planBuilder) buildSelect(sel *ast.SelectStmt) Plan {
var aggFuncs []*ast.AggregateFuncExpr
if b.detectSelectAgg(sel) {
hasAgg := b.detectSelectAgg(sel)
if hasAgg {
aggFuncs = b.extractSelectAgg(sel)
}
var p Plan
Expand All @@ -152,15 +153,15 @@ func (b *planBuilder) buildSelect(sel *ast.SelectStmt) Plan {
return nil
}
}
if len(aggFuncs) > 0 || sel.GroupBy != nil {
if hasAgg {
p = b.buildAggregate(p, aggFuncs, sel.GroupBy)
}
p = b.buildSelectFields(p, sel.GetResultFields())
if b.err != nil {
return nil
}
} else {
if len(aggFuncs) > 0 {
if hasAgg {
p = b.buildAggregate(p, aggFuncs, nil)
}
p = b.buildSelectFields(p, sel.GetResultFields())
Expand Down
9 changes: 4 additions & 5 deletions optimizer/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,15 +308,16 @@ func (nr *nameResolver) resolveColumnNameInContext(ctx *resolverContext, cn *ast
}
return nr.resolveColumnInResultFields(cn, ctx.fieldList)
}
// Resolve from table first, than from select list.
// Resolve from table first, then from select list.
found := nr.resolveColumnInTableSources(cn, ctx.tables)
if nr.Err != nil {
return found
}
// We should copy the refer here.
// Because if the ByItem is an identifier, we should check if it
// is ambiguous even it is already resolved from table source.
// If the ByItem is not an identifier, we do not need the second check.
r := cn.Refer
pe := nr.Err
nr.Err = nil
if nr.resolveColumnInResultFields(cn, ctx.fieldList) {
if nr.Err != nil {
return true
Expand All @@ -328,8 +329,6 @@ func (nr *nameResolver) resolveColumnNameInContext(ctx *resolverContext, cn *ast
}
return true
}
// Restore err.
nr.Err = pe
return found
}
if ctx.inHaving {
Expand Down

0 comments on commit 7da9b26

Please sign in to comment.