Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opt: pass Const type explicitly #48652

Merged
merged 1 commit into from
May 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/apply_join
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ PREPARE b AS OPT PLAN '
(Scan [(Table "t") (Cols "k,str") ])
(Select
(Scan [(Table "u") (Cols "l,str2") ])
[ (Eq (Plus (Var "k") (Const 1)) (Var "l") )]
[ (Eq (Plus (Var "k") (Const 1 "int")) (Var "l") )]
)
[]
[]
Expand All @@ -72,7 +72,7 @@ PREPARE c AS OPT PLAN '
(Scan [(Table "t") (Cols "k,str") ])
(Select
(Scan [(Table "u") (Cols "l,str2") ])
[ (Eq (Plus (Var "k") (Const 1)) (Var "l") )]
[ (Eq (Plus (Var "k") (Const 1 "int")) (Var "l") )]
)
[]
[]
Expand All @@ -98,7 +98,7 @@ PREPARE d AS OPT PLAN '
(Scan [(Table "t") (Cols "k,str") ])
(Select
(Scan [(Table "u") (Cols "l,str2") ])
[ (Eq (Plus (Var "k") (Const 1)) (Var "l") )]
[ (Eq (Plus (Var "k") (Const 1 "int")) (Var "l") )]
)
[]
[]
Expand Down Expand Up @@ -158,7 +158,7 @@ PREPARE f AS OPT PLAN '
(Select
(Scan [(Table "u") (Cols "l,str2") ])
[ (Eq (Plus (Var "k")
(Subquery (Values [(Tuple [(Const 1)] "tuple{int}") ]
(Subquery (Values [(Tuple [(Const 1 "int")] "tuple{int}") ]
[(Cols [(NewColumn "z" "int")] )])
[]))
(Var "l") )]
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/prepare
Original file line number Diff line number Diff line change
Expand Up @@ -1095,8 +1095,8 @@ PREPARE b AS OPT PLAN '
(Scan [ (Table "t2") (Cols "k,str") ])
[
(Eq
(Mod (Var "k") (Const 2))
(Const 1)
(Mod (Var "k") (Const 2 "int"))
(Const 1 "int")
)
]
)
Expand All @@ -1120,8 +1120,8 @@ PREPARE e AS OPT PLAN '
(Scan [ (Table "t2") (Cols "k,str") ])
[
(Eq
(Mod (Var "k") (Const 2))
(Const 1)
(Mod (Var "k") (Const 2 "int"))
(Const 1 "int")
)
]
)
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/memo/testdata/stats/join
Original file line number Diff line number Diff line change
Expand Up @@ -1425,7 +1425,7 @@ full-join (cross)
expr
(SemiJoin
(Values
[ (Tuple [ (Const 1) (Const 2) ] "tuple{int}" ) ]
[ (Tuple [ (Const 1 "int") (Const 2 "int") ] "tuple{int}" ) ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these be MakeIntConst?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there are no custom functions in exprgen

[ (Cols [ (NewColumn "a" "int") (NewColumn "b" "int") ]) ]
)
(Scan [ (Table "uv") (Cols "u,v,rowid") ])
Expand Down Expand Up @@ -1456,7 +1456,7 @@ semi-join (cross)
expr
(AntiJoin
(Values
[ (Tuple [ (Const 1) (Const 2) ] "tuple{int}" ) ]
[ (Tuple [ (Const 1 "int") (Const 2 "int") ] "tuple{int}" ) ]
[ (Cols [ (NewColumn "a" "int") (NewColumn "b" "int") ]) ]
)
(Scan [ (Table "uv") (Cols "u,v,rowid") ])
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/memo/testdata/stats/select
Original file line number Diff line number Diff line change
Expand Up @@ -1264,7 +1264,7 @@ expr colstat=1 colstat=2
]`)
]
)
[ (Eq (Var "b") (Const 1)) ]
[ (Eq (Var "b") (Const 1 "int")) ]
)
----
select
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/opt/memo/typing.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ var typingFuncMap map[opt.Operator]typingFunc

func init() {
typingFuncMap = make(map[opt.Operator]typingFunc)
typingFuncMap[opt.ConstOp] = typeAsTypedExpr
typingFuncMap[opt.PlaceholderOp] = typeAsTypedExpr
typingFuncMap[opt.UnsupportedExprOp] = typeAsTypedExpr
typingFuncMap[opt.CoalesceOp] = typeCoalesce
Expand Down
18 changes: 12 additions & 6 deletions pkg/sql/opt/norm/custom_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ func (c *CustomFuncs) DerefOrderingChoice(result *physical.OrderingChoice) physi
return *result
}

// IntConst constructs a Const holding a DInt.
func (c *CustomFuncs) IntConst(d *tree.DInt) opt.ScalarExpr {
return c.f.ConstructConst(d, types.Int)
}

// ----------------------------------------------------------------------
//
// ScalarList functions
Expand Down Expand Up @@ -2208,7 +2213,7 @@ func (c *CustomFuncs) MakeSingleKeyJSONObject(key, value opt.ScalarExpr) opt.Sca
builder.Add(string(*k), v.JSON)
j := builder.Build()

return c.f.ConstructConst(&tree.DJSON{JSON: j})
return c.f.ConstructConst(&tree.DJSON{JSON: j}, types.Jsonb)
}

// IsConstValueEqual returns whether const1 and const2 are equal.
Expand Down Expand Up @@ -2348,7 +2353,7 @@ func (c *CustomFuncs) CastToCollatedString(str opt.ScalarExpr, locale string) op
if err != nil {
panic(err)
}
return c.f.ConstructConst(d)
return c.f.ConstructConst(d, types.MakeCollatedString(str.DataType(), locale))
}

// MakeUnorderedSubquery returns a SubqueryPrivate that specifies no ordering.
Expand Down Expand Up @@ -2568,16 +2573,17 @@ func (c *CustomFuncs) EqualsNumber(datum tree.Datum, value int64) bool {
return false
}

// AddConstInts adds the numeric constants together. AddConstInts assumes the sum
// will not overflow. Call CanAddConstInts on the constants to guarantee this.
func (c *CustomFuncs) AddConstInts(first tree.Datum, second tree.Datum) tree.Datum {
// AddConstInts adds the numeric constants together and constructs a Const.
// AddConstInts assumes the sum will not overflow. Call CanAddConstInts on the
// constants to guarantee this.
func (c *CustomFuncs) AddConstInts(first tree.Datum, second tree.Datum) opt.ScalarExpr {
firstVal := int64(*first.(*tree.DInt))
secondVal := int64(*second.(*tree.DInt))
sum, ok := arith.AddWithOverflow(firstVal, secondVal)
if !ok {
panic(errors.AssertionFailedf("addition of %d and %d overflowed", firstVal, secondVal))
}
return tree.NewDInt(tree.DInt(sum))
return c.f.ConstructConst(tree.NewDInt(tree.DInt(sum)), types.Int)
}

// CanAddConstInts returns true if the addition of the two integers overflows.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/norm/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,5 +340,5 @@ func (f *Factory) ConstructConstVal(d tree.Datum, t *types.T) opt.ScalarExpr {
}
return memo.FalseSingleton
}
return f.ConstructConst(d)
return f.ConstructConst(d, t)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just kill this method altogether, and then add a couple rules for converting (Const Null) to (Null), (Const True) to (True), etc?

}
2 changes: 1 addition & 1 deletion pkg/sql/opt/norm/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestSimplifyFilters(t *testing.T) {
ax := a.ColumnID(0)

variable := f.ConstructVariable(ax)
constant := f.ConstructConst(tree.NewDInt(1))
constant := f.ConstructConst(tree.NewDInt(1), types.Int)
eq := f.ConstructEq(variable, constant)

// Filters expression evaluates to False if any operand is False.
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/norm/fold_constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (c *CustomFuncs) FoldArray(elems memo.ScalarListExpr, typ *types.T) opt.Sca
a.HasNonNulls = true
}
}
return c.f.ConstructConst(a)
return c.f.ConstructConst(a, typ)
}

// IsConstValueOrTuple returns true if the input is a constant or a tuple of
Expand Down Expand Up @@ -237,7 +237,7 @@ func (c *CustomFuncs) UnifyComparison(left, right opt.ScalarExpr) opt.ScalarExpr
return nil
}

return c.f.ConstructConst(convertedDatum)
return c.f.ConstructConst(convertedDatum, desiredType)
}

// FoldComparison evaluates a comparison expression with constant inputs. It
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/norm/rules/groupby.opt
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@
(ConstructProjectionFromDistinctOn
(Limit
$input
(Const 1)
(IntConst 1)
(GroupingInputOrdering $groupingPrivate)
)
(MakeEmptyColSet)
Expand Down
16 changes: 6 additions & 10 deletions pkg/sql/opt/norm/rules/limit.opt
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ $input
(Limit
(Offset
$input:*
(Const $offset:* & (IsPositiveLimit $offset))
$offsetExpr:(Const $offset:* & (IsPositiveLimit $offset))
$offsetOrdering:*
)
(Const $limit:* & (IsPositiveLimit $limit))
Expand All @@ -76,12 +76,8 @@ $input
)
=>
(Offset
(Limit
$input
(Const (AddConstInts $offset $limit))
$limitOrdering
)
(Const $offset)
(Limit $input (AddConstInts $offset $limit) $limitOrdering)
$offsetExpr
$offsetOrdering
)

Expand Down Expand Up @@ -132,7 +128,7 @@ $input
[PushLimitIntoLeftJoin, Normalize]
(Limit
$input:(LeftJoin $left:* $right:* $on:* $private:*)
(Const $limit:*) &
$limitExpr:(Const $limit:*) &
(IsPositiveLimit $limit) &
^(LimitGeMaxRows $limit $left)
$ordering:* & (HasColsInOrdering $left $ordering)
Expand All @@ -142,13 +138,13 @@ $input
(LeftJoin
(Limit
$left
(Const $limit)
$limitExpr
(PruneOrdering $ordering (OutputCols $left))
)
$right
$on
$private
)
(Const $limit)
$limitExpr
$ordering
)
2 changes: 1 addition & 1 deletion pkg/sql/opt/norm/rules/scalar.opt
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ $input
)
=>
(Exists
(Limit $input (Const 1) (EmptyOrdering))
(Limit $input (IntConst 1) (EmptyOrdering))
(MakeLimited $subqueryPrivate)
)

Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/opt/norm/testdata/rules/reject_nulls
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ exprnorm expect=RejectNullsGroupBy
]
[ ]
)
[ (Eq (Var "sum") (Const 10)) ]
[ (Eq (Var "sum") (Const 10 "int")) ]
)
(Presentation "u,v")
(NoOrdering)
Expand Down Expand Up @@ -690,7 +690,7 @@ exprnorm
[ (Tuple [ (Plus (Var "x") (Var "u")) ] "tuple{int}" ) ]
[ (Cols [ (NewColumn "z" "int") ]) ]
)
[ (Eq (Var "x") (Const 3)) ]
[ (Eq (Var "x") (Const 3 "int")) ]
)
[ ]
[ ]
Expand All @@ -701,7 +701,7 @@ exprnorm
[ (AggregationsItem (Sum (Var "z")) (NewColumn "sum" "int")) ]
[ ]
)
[ (Eq (Var "sum") (Const 10)) ]
[ (Eq (Var "sum") (Const 10 "int")) ]
)
----
select
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/opt/ops/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ operator can contain, in this order:
the `Private` tag). The private fields are interned with the expression and
can be used by rules; they must be initialized before construction of the
expression. Private fields can be accessed by rules.
- at most one `Typ` field (only for Scalar operators). If this field is not
present, the scalar type of the operator is inferred from its inputs.
- any number of unexported fields. Unexported fields are typically used to
cache information that can be deduced from the children and the private. If
there are unexported fields, an `initUnexportedFields(*Memo)` method must be
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/opt/ops/scalar.opt
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ define Variable {
[Scalar, ConstValue]
define Const {
Value Datum

# Typ is the type of the constant. It is necessary because
# Value.ResolvedType() loses information in some cases.
Typ Type
}

# Null is the constant SQL null value that has "unknown value" semantics. If
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/optbuilder/window.go
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ func (b *Builder) replaceDefaultReturn(
func (b *Builder) overrideDefaultNullValue(agg aggregateInfo) (opt.ScalarExpr, bool) {
switch agg.def.Name {
case "count", "count_rows":
return b.factory.ConstructConst(tree.NewDInt(0)), true
return b.factory.ConstructConst(tree.NewDInt(0), types.Int), true
default:
return nil, false
}
Expand Down
6 changes: 1 addition & 5 deletions pkg/sql/opt/optgen/cmd/optgen/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,7 @@ func (m *metadata) childAndPrivateFields(define *lang.DefineExpr) lang.DefineFie
n := 0
for _, field := range define.Fields {
typ := m.typeOf(field)
if !typ.isExpr {
// If this is the private field, include it.
if isEmbeddedField(field) || isExportedField(field) {
n++
}
if !typ.isExpr && !isEmbeddedField(field) && !isExportedField(field) {
break
}
n++
Expand Down
18 changes: 11 additions & 7 deletions pkg/sql/opt/optgen/cmd/optgen/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,19 @@ func (v *validator) validate(compiled *lang.CompiledExpr) []error {
exprsDone = true

if isExportedField(field) || isEmbeddedField(field) {
// Private definition.
if privateDone {
format := "private field '%s' cannot follow private or unexported field in '%s'"
v.addErrorf(field.Source(), format, field.Name, define.Name)
break
// Tolerate a Typ field for Scalars (even if there was a Private
// field).
if !(define.Tags.Contains("Scalar") && field.Name == "Typ" && field.Type == "Type") {
// Private definition.
if privateDone {
format := "private field '%s' cannot follow private or unexported field in '%s'"
v.addErrorf(field.Source(), format, field.Name, define.Name)
break
}
}
}
// This is either a private definition, or an unexported field. In either
// case, we can no longer accept a private definition.
// This is either a private definition, a Typ field, or an unexported
// field. In either case, we can no longer accept a private definition.
privateDone = true
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/optgen/exprgen/testdata/limit
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ CREATE TABLE abc (a INT, b INT, c INT, INDEX ab(a, b))
expr
(Limit
(Scan [ (Table "abc") (Index "abc@ab") (Cols "a,b") ])
(Const 10)
(Const 10 "int")
(OrderingChoice "+a")
)
----
Expand All @@ -30,7 +30,7 @@ limit
expr
(Limit
(Sort (Scan [ (Table "abc") (Cols "a,b") ]))
(Const 10)
(Const 10 "int")
(OrderingChoice "+a")
)
----
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/optgen/exprgen/testdata/scalar
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ eq [type=bool]
└── false [type=bool]

expr
(Plus (Const 1) (Const 2))
(Plus (Const 1 "int") (Const 2 "int"))
----
plus [type=int]
├── const: 1 [type=int]
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/optgen/exprgen/testdata/scan
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ scan t.public.abc@ab
expr
(Select
(Scan [ (Table "abc") (Cols "a,b,c") ])
[ (Eq (Var "a") (Const 1)) ]
[ (Eq (Var "a") (Const 1 "int")) ]
)
----
select
Expand Down
Loading