Skip to content
This repository has been archived by the owner on Jan 28, 2021. It is now read-only.

Commit

Permalink
Merge pull request #609 from mcarmonaa/fix/alias-same-projection
Browse files Browse the repository at this point in the history
sql/analyzer: fix use of declared aliases in the same projection
  • Loading branch information
ajnavarro authored Jan 30, 2019
2 parents 5bbb4f0 + 9ec9c7b commit 31ad0f9
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 24 deletions.
90 changes: 73 additions & 17 deletions sql/analyzer/resolve_columns.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,79 @@ import (
"gopkg.in/src-d/go-vitess.v1/vt/sqlparser"
)

func checkAliases(ctx *sql.Context, a *Analyzer, n sql.Node) (sql.Node, error) {
span, _ := ctx.Span("check_aliases")
defer span.Finish()

a.Log("check aliases")

var err error
plan.Inspect(n, func(node sql.Node) bool {
p, ok := node.(*plan.Project)
if !ok {
return true
}

aliases := lookForAliasDeclarations(p)
for alias := range aliases {
if isAliasUsed(p, alias) {
err = ErrMisusedAlias.New(alias)
}
}

return true
})

return n, err
}

func lookForAliasDeclarations(node sql.Expressioner) map[string]struct{} {
var (
aliases = map[string]struct{}{}
in = struct{}{}
)

for _, e := range node.Expressions() {
expression.Inspect(e, func(expr sql.Expression) bool {
if alias, ok := expr.(*expression.Alias); ok {
aliases[alias.Name()] = in
}

return true
})
}

return aliases
}

func isAliasUsed(node sql.Expressioner, alias string) bool {
var found bool
for _, e := range node.Expressions() {
expression.Inspect(e, func(expr sql.Expression) bool {
if a, ok := expr.(*expression.Alias); ok {
if a.Name() == alias {
return false
}

return true
}

if n, ok := expr.(sql.Nameable); ok && n.Name() == alias {
found = true
return false
}

return true
})

if found {
break
}
}

return found
}

// deferredColumn is a wrapper on UnresolvedColumn used only to defer the
// resolution of the column because it may require some work done by
// other analyzer phases.
Expand Down Expand Up @@ -221,18 +294,6 @@ func resolveColumns(ctx *sql.Context, a *Analyzer, n sql.Node) (sql.Node, error)
}
}

var (
aliasMap = make(map[string]struct{})
exists = struct{}{}
)
if project, ok := n.(*plan.Project); ok {
for _, e := range project.Projections {
if alias, ok := e.(*expression.Alias); ok {
aliasMap[strings.ToLower(alias.Name())] = exists
}
}
}

expressioner, ok := n.(sql.Expressioner)
if !ok {
return n, nil
Expand Down Expand Up @@ -287,11 +348,6 @@ func resolveColumns(ctx *sql.Context, a *Analyzer, n sql.Node) (sql.Node, error)
return nil, ErrColumnTableNotFound.New(uc.Table(), uc.Name())
}

if _, ok := aliasMap[name]; ok {
// no nested aliases
return nil, ErrMisusedAlias.New(uc.Name())
}

return nil, ErrColumnNotFound.New(uc.Name())
}
}
Expand Down
9 changes: 2 additions & 7 deletions sql/analyzer/resolve_columns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestQualifyColumnsProject(t *testing.T) {

func TestMisusedAlias(t *testing.T) {
require := require.New(t)
f := getRule("resolve_columns")
f := getRule("check_aliases")

table := mem.NewTable("mytable", sql.Schema{
{Name: "i", Type: sql.Int32},
Expand All @@ -71,12 +71,7 @@ func TestMisusedAlias(t *testing.T) {
plan.NewResolvedTable(table),
)

// the first iteration wrap the unresolved column "alias_i" as a maybeAlias
n, err := f.Apply(sql.NewEmptyContext(), nil, node)
require.NoError(err)

// if maybeAlias is not resolved it fails
_, err = f.Apply(sql.NewEmptyContext(), nil, n)
_, err := f.Apply(sql.NewEmptyContext(), nil, node)
require.EqualError(err, ErrMisusedAlias.New("alias_i").Error())
}

Expand Down
1 change: 1 addition & 0 deletions sql/analyzer/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var DefaultRules = []Rule{
var OnceBeforeDefault = []Rule{
{"resolve_subqueries", resolveSubqueries},
{"resolve_tables", resolveTables},
{"check_aliases", checkAliases},
}

// OnceAfterDefault contains the rules to be applied just once after the
Expand Down

0 comments on commit 31ad0f9

Please sign in to comment.