diff --git a/plan/plans/select_list.go b/plan/plans/select_list.go index bacc7b729d..66704acdfb 100644 --- a/plan/plans/select_list.go +++ b/plan/plans/select_list.go @@ -124,24 +124,6 @@ func (s *SelectList) UpdateAggFields(expr expression.Expression) (expression.Exp return &expression.Position{N: index + 1, Name: name}, nil } -// CloneHiddenField checks and clones field and result field from table fields, -// and adds them to hidden field of select list. -func (s *SelectList) CloneHiddenField(name string, tableFields []*field.ResultField) bool { - // Check and add hidden field. - if field.ContainFieldName(name, tableFields, field.CheckFieldFlag) { - resultField, _ := field.CloneFieldByName(name, tableFields, field.CheckFieldFlag) - f := &field.Field{ - Expr: &expression.Ident{ - CIStr: resultField.ColumnInfo.Name, - }, - } - s.AddField(f, resultField) - return true - } - - return false -} - // CheckReferAmbiguous checks whether an identifier reference is ambiguous or not in select list. // e,g, "select c1 as a, c2 as a from t group by a" is ambiguous, // but "select c1 as a, c1 as a from t group by a" is not. @@ -235,6 +217,7 @@ func ResolveSelectList(selectFields []*field.Field, srcFields []*field.ResultFie continue } + // TODO: use fromIdentVisitor to cleanup. var result *field.ResultField for _, name := range names { idx := field.GetResultFieldIndex(name, srcFields, field.DefaultFieldFlag) diff --git a/rset/rsets/fields.go b/rset/rsets/fields.go index 4df4726463..ffe6176409 100644 --- a/rset/rsets/fields.go +++ b/rset/rsets/fields.go @@ -40,7 +40,7 @@ type SelectFieldsRset struct { } func updateSelectFieldsRefer(selectList *plans.SelectList) error { - visitor := newFromIdentVisitor(selectList.FromFields) + visitor := newFromIdentVisitor(selectList.FromFields, fieldListClause) // we only fix un-hidden fields here, for hidden fields, it should be // handled in their own clause, in order by or having. diff --git a/rset/rsets/helper.go b/rset/rsets/helper.go index abea6bb44f..5f54d9f435 100644 --- a/rset/rsets/helper.go +++ b/rset/rsets/helper.go @@ -50,13 +50,22 @@ type clauseType int const ( noneClause clauseType = iota + onClause + whereClause groupByClause - orderByClause + fieldListClause havingClause + orderByClause ) func (clause clauseType) String() string { switch clause { + case onClause: + return "on clause" + case fieldListClause: + return "field list" + case whereClause: + return "where clause" case groupByClause: return "group statement" case orderByClause: @@ -138,24 +147,28 @@ func checkIdent(i *expression.Ident, selectList *plans.SelectList, clause clause type fromIdentVisitor struct { expression.BaseVisitor fromFields []*field.ResultField + clause clauseType } func (v *fromIdentVisitor) VisitIdent(i *expression.Ident) (expression.Expression, error) { idx := field.GetResultFieldIndex(i.L, v.fromFields, field.DefaultFieldFlag) - if len(idx) > 0 { + if len(idx) == 1 { i.ReferScope = expression.IdentReferFromTable i.ReferIndex = idx[0] return i, nil + } else if len(idx) > 1 { + return nil, errors.Errorf("Column '%s' in %s is ambiguous", i, v.clause) } // TODO: check in outer query return i, nil } -func newFromIdentVisitor(fromFields []*field.ResultField) *fromIdentVisitor { +func newFromIdentVisitor(fromFields []*field.ResultField, clause clauseType) *fromIdentVisitor { visitor := &fromIdentVisitor{} visitor.BaseVisitor.V = visitor visitor.fromFields = fromFields + visitor.clause = clause return visitor } diff --git a/rset/rsets/where.go b/rset/rsets/where.go index f32313ac8f..3a583fe010 100644 --- a/rset/rsets/where.go +++ b/rset/rsets/where.go @@ -180,7 +180,7 @@ func (r *WhereRset) planStatic(ctx context.Context, e expression.Expression) (pl } func (r *WhereRset) updateWhereFieldsRefer() error { - visitor := newFromIdentVisitor(r.Src.GetFields()) + visitor := newFromIdentVisitor(r.Src.GetFields(), whereClause) e, err := r.Expr.Accept(visitor) if err != nil {