From 7da9b268e5554ae805e89d2dec7f2db3d5caebbc Mon Sep 17 00:00:00 2001 From: shenli Date: Thu, 14 Jan 2016 16:00:08 +0800 Subject: [PATCH] *: Address comment --- optimizer/plan/planbuilder.go | 9 +++++---- optimizer/resolver.go | 9 ++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/optimizer/plan/planbuilder.go b/optimizer/plan/planbuilder.go index 11fb544365..ab33dee9cd 100644 --- a/optimizer/plan/planbuilder.go +++ b/optimizer/plan/planbuilder.go @@ -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) @@ -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 @@ -152,7 +153,7 @@ 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()) @@ -160,7 +161,7 @@ func (b *planBuilder) buildSelect(sel *ast.SelectStmt) Plan { return nil } } else { - if len(aggFuncs) > 0 { + if hasAgg { p = b.buildAggregate(p, aggFuncs, nil) } p = b.buildSelectFields(p, sel.GetResultFields()) diff --git a/optimizer/resolver.go b/optimizer/resolver.go index ed808a9011..595423f273 100644 --- a/optimizer/resolver.go +++ b/optimizer/resolver.go @@ -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 @@ -328,8 +329,6 @@ func (nr *nameResolver) resolveColumnNameInContext(ctx *resolverContext, cn *ast } return true } - // Restore err. - nr.Err = pe return found } if ctx.inHaving {