Skip to content

Commit

Permalink
sql: name<->string type equality
Browse files Browse the repository at this point in the history
This commit allows TypeString and TypeName to compare to each other via
`Equals` but not via `FamilyEquals`, and adds an extra rule to the
overload finder that attempts to narrow the list of overloads by
`FamilyEquals`-based equality.

This allows us to safely remove the comparison overloads for operations
between names and strings.
  • Loading branch information
jordanlewis committed Dec 12, 2016
1 parent 3278faa commit f869548
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 46 deletions.
1 change: 1 addition & 0 deletions pkg/sql/parser/aggregate_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ type AggregateFunc interface {
// Exported for use in documentation.
var Aggregates = map[string][]Builtin{
"array_agg": {
// TODO(knz) Collapse these into a single aggregate.
makeAggBuiltin(TypeInt, TypeIntArray, newIntArrayAggregate),
makeAggBuiltin(TypeString, TypeStringArray, newStringArrayAggregate),
makeAggBuiltin(TypeName, TypeNameArray, newNameArrayAggregate),
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/parser/constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@ func (expr *StrVal) ResolveAsType(ctx *SemaContext, typ Type) (Datum, error) {
case TypeString:
expr.resString = DString(expr.s)
return &expr.resString, nil
case TypeName:
return NewDName(expr.s), nil
case TypeBytes:
expr.resBytes = DBytes(expr.s)
return &expr.resBytes, nil
Expand Down
4 changes: 3 additions & 1 deletion pkg/sql/parser/datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,9 @@ func (d *DName) IsMax() bool { return d.DString.IsMax() }
func (d *DName) IsMin() bool { return d.DString.IsMin() }

// Size implements the Datum interface.
func (d *DName) Size() uintptr { return d.DString.Size() }
func (d *DName) Size() uintptr {
return unsafe.Sizeof(*d) + uintptr(len(d.DString))
}

// min implements the Datum interface.
func (d *DName) min() (Datum, bool) { return nameify(d.DString.min()) }
Expand Down
42 changes: 0 additions & 42 deletions pkg/sql/parser/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -903,20 +903,6 @@ var CmpOps = map[ComparisonOperator]cmpOpOverload{
return DBool(bytes.Equal(left.(*DCollatedString).key, right.(*DCollatedString).key)), nil
},
},
CmpOp{
LeftType: TypeName,
RightType: TypeString,
fn: func(_ *EvalContext, left Datum, right Datum) (DBool, error) {
return DBool(left.(*DName).DString == *right.(*DString)), nil
},
},
CmpOp{
LeftType: TypeString,
RightType: TypeName,
fn: func(_ *EvalContext, left Datum, right Datum) (DBool, error) {
return DBool(*left.(*DString) == right.(*DName).DString), nil
},
},
CmpOp{
LeftType: TypeBytes,
RightType: TypeBytes,
Expand Down Expand Up @@ -1388,20 +1374,6 @@ var CmpOps = map[ComparisonOperator]cmpOpOverload{
return matchLike(ctx, left, right, false)
},
},
CmpOp{
LeftType: TypeName,
RightType: TypeString,
fn: func(ctx *EvalContext, left Datum, right Datum) (DBool, error) {
return matchLike(ctx, left, right, false)
},
},
CmpOp{
LeftType: TypeString,
RightType: TypeName,
fn: func(ctx *EvalContext, left Datum, right Datum) (DBool, error) {
return matchLike(ctx, left, right, false)
},
},
},

ILike: {
Expand All @@ -1412,20 +1384,6 @@ var CmpOps = map[ComparisonOperator]cmpOpOverload{
return matchLike(ctx, left, right, true)
},
},
CmpOp{
LeftType: TypeName,
RightType: TypeString,
fn: func(ctx *EvalContext, left Datum, right Datum) (DBool, error) {
return matchLike(ctx, left, right, true)
},
},
CmpOp{
LeftType: TypeString,
RightType: TypeName,
fn: func(ctx *EvalContext, left Datum, right Datum) (DBool, error) {
return matchLike(ctx, left, right, true)
},
},
},

SimilarTo: {
Expand Down
15 changes: 14 additions & 1 deletion pkg/sql/parser/overload.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func (AnyType) matchLen(l int) bool {
}

func (AnyType) getAt(i int) Type {
panic("getAt called on AnyType")
return TypeAny
}

// Length implements the typeList interface.
Expand Down Expand Up @@ -421,6 +421,16 @@ func typeCheckOverloadedExprs(
return typedExprs, fn, err
}

// Further narrow the list by TypeFamily.
for _, expr := range resolvableExprs {
filterOverloads(func(o overloadImpl) bool {
return o.params().getAt(expr.i).FamilyEqual(typedExprs[expr.i].ResolvedType())
})
}
if ok, fn, err := checkReturn(); ok {
return typedExprs, fn, err
}

// The first heuristic is to prefer candidates that return the desired type.
if desired != TypeAny {
filterOverloads(func(o overloadImpl) bool {
Expand Down Expand Up @@ -538,5 +548,8 @@ func typeCheckOverloadedExprs(
preferred = c
}
}
if preferred == nil {
return nil, nil, fmt.Errorf("ambiguous typecheck result: %d matches: %s", len(overloads), overloads)
}
return typedExprs, preferred, nil
}
4 changes: 2 additions & 2 deletions pkg/sql/parser/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ type tString struct{}

func (tString) String() string { return "string" }
func (tString) Equal(other Type) bool {
return other == TypeString || other == TypeAny
return other == TypeString || other == TypeAny || other == TypeName
}
func (tString) FamilyEqual(other Type) bool { return other == TypeString }
func (tString) Size() (uintptr, bool) { return unsafe.Sizeof(DString("")), variableSize }
Expand Down Expand Up @@ -203,7 +203,7 @@ type tName struct{}

func (tName) String() string { return "name" }
func (tName) Equal(other Type) bool {
return other == TypeName || other == TypeAny
return other == TypeString || other == TypeAny || other == TypeName
}
func (tName) FamilyEqual(other Type) bool { return other == TypeName }
func (tName) Size() (uintptr, bool) { return unsafe.Sizeof(DBytes("")), variableSize }
Expand Down

0 comments on commit f869548

Please sign in to comment.