diff --git a/pkg/sql/logictest/testdata/logic_test/apply_join b/pkg/sql/logictest/testdata/logic_test/apply_join index dc4c016c36b0..c49cef2608a5 100644 --- a/pkg/sql/logictest/testdata/logic_test/apply_join +++ b/pkg/sql/logictest/testdata/logic_test/apply_join @@ -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") )] ) [] [] @@ -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") )] ) [] [] @@ -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") )] ) [] [] @@ -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") )] diff --git a/pkg/sql/logictest/testdata/logic_test/prepare b/pkg/sql/logictest/testdata/logic_test/prepare index 5d35797cca44..19a550190ad5 100644 --- a/pkg/sql/logictest/testdata/logic_test/prepare +++ b/pkg/sql/logictest/testdata/logic_test/prepare @@ -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") ) ] ) @@ -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") ) ] ) diff --git a/pkg/sql/opt/memo/testdata/stats/join b/pkg/sql/opt/memo/testdata/stats/join index 636f18ff6ebe..4e50f4c66bac 100644 --- a/pkg/sql/opt/memo/testdata/stats/join +++ b/pkg/sql/opt/memo/testdata/stats/join @@ -1420,7 +1420,7 @@ full-join (cross) expr (SemiJoin (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") ]) @@ -1451,7 +1451,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") ]) diff --git a/pkg/sql/opt/memo/testdata/stats/select b/pkg/sql/opt/memo/testdata/stats/select index 151af4b7a436..80b4c2eb970b 100644 --- a/pkg/sql/opt/memo/testdata/stats/select +++ b/pkg/sql/opt/memo/testdata/stats/select @@ -1264,7 +1264,7 @@ expr colstat=1 colstat=2 ]`) ] ) - [ (Eq (Var "b") (Const 1)) ] + [ (Eq (Var "b") (Const 1 "int")) ] ) ---- select diff --git a/pkg/sql/opt/memo/typing.go b/pkg/sql/opt/memo/typing.go index abd875a9d0bc..111c32af84e5 100644 --- a/pkg/sql/opt/memo/typing.go +++ b/pkg/sql/opt/memo/typing.go @@ -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 diff --git a/pkg/sql/opt/norm/custom_funcs.go b/pkg/sql/opt/norm/custom_funcs.go index d9d0e5e700ba..b35377267e02 100644 --- a/pkg/sql/opt/norm/custom_funcs.go +++ b/pkg/sql/opt/norm/custom_funcs.go @@ -59,6 +59,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 @@ -1744,7 +1749,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. @@ -1864,7 +1869,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. @@ -2009,16 +2014,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. diff --git a/pkg/sql/opt/norm/factory.go b/pkg/sql/opt/norm/factory.go index cc7327cf5cf6..bc23bf8511a5 100644 --- a/pkg/sql/opt/norm/factory.go +++ b/pkg/sql/opt/norm/factory.go @@ -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) } diff --git a/pkg/sql/opt/norm/factory_test.go b/pkg/sql/opt/norm/factory_test.go index b1c9e59753d6..bdbe52d68dd1 100644 --- a/pkg/sql/opt/norm/factory_test.go +++ b/pkg/sql/opt/norm/factory_test.go @@ -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. diff --git a/pkg/sql/opt/norm/fold_constants.go b/pkg/sql/opt/norm/fold_constants.go index de9888225782..524108b2a4bf 100644 --- a/pkg/sql/opt/norm/fold_constants.go +++ b/pkg/sql/opt/norm/fold_constants.go @@ -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 @@ -240,7 +240,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 diff --git a/pkg/sql/opt/norm/rules/groupby.opt b/pkg/sql/opt/norm/rules/groupby.opt index a9b152e47a56..469bd33fe377 100644 --- a/pkg/sql/opt/norm/rules/groupby.opt +++ b/pkg/sql/opt/norm/rules/groupby.opt @@ -175,7 +175,7 @@ (ConstructProjectionFromDistinctOn (Limit $input - (Const 1) + (IntConst 1) (GroupingInputOrdering $groupingPrivate) ) (MakeEmptyColSet) diff --git a/pkg/sql/opt/norm/rules/limit.opt b/pkg/sql/opt/norm/rules/limit.opt index 6044fd31cf9f..a540071e8fd7 100644 --- a/pkg/sql/opt/norm/rules/limit.opt +++ b/pkg/sql/opt/norm/rules/limit.opt @@ -66,7 +66,7 @@ $input (Limit (Offset $input:* - (Const $offset:* & (IsPositiveLimit $offset)) + $offsetExpr:(Const $offset:* & (IsPositiveLimit $offset)) $offsetOrdering:* ) (Const $limit:* & (IsPositiveLimit $limit)) @@ -76,12 +76,8 @@ $input ) => (Offset - (Limit - $input - (Const (AddConstInts $offset $limit)) - $limitOrdering - ) - (Const $offset) + (Limit $input (AddConstInts $offset $limit) $limitOrdering) + $offsetExpr $offsetOrdering ) @@ -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) @@ -142,13 +138,13 @@ $input (LeftJoin (Limit $left - (Const $limit) + $limitExpr (PruneOrdering $ordering (OutputCols $left)) ) $right $on $private ) - (Const $limit) + $limitExpr $ordering ) diff --git a/pkg/sql/opt/norm/rules/scalar.opt b/pkg/sql/opt/norm/rules/scalar.opt index 53935494824f..1f8483b18abf 100644 --- a/pkg/sql/opt/norm/rules/scalar.opt +++ b/pkg/sql/opt/norm/rules/scalar.opt @@ -130,7 +130,7 @@ $input ) => (Exists - (Limit $input (Const 1) (EmptyOrdering)) + (Limit $input (IntConst 1) (EmptyOrdering)) (MakeLimited $subqueryPrivate) ) diff --git a/pkg/sql/opt/norm/testdata/rules/reject_nulls b/pkg/sql/opt/norm/testdata/rules/reject_nulls index b1a3314250b7..2ab0cc582c0f 100644 --- a/pkg/sql/opt/norm/testdata/rules/reject_nulls +++ b/pkg/sql/opt/norm/testdata/rules/reject_nulls @@ -412,7 +412,7 @@ exprnorm expect=RejectNullsGroupBy ] [ ] ) - [ (Eq (Var "sum") (Const 10)) ] + [ (Eq (Var "sum") (Const 10 "int")) ] ) (Presentation "u,v") (NoOrdering) @@ -696,7 +696,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")) ] ) [ ] [ ] @@ -707,7 +707,7 @@ exprnorm [ (AggregationsItem (Sum (Var "z")) (NewColumn "sum" "int")) ] [ ] ) - [ (Eq (Var "sum") (Const 10)) ] + [ (Eq (Var "sum") (Const 10 "int")) ] ) ---- select diff --git a/pkg/sql/opt/ops/README.md b/pkg/sql/opt/ops/README.md index 122b979e8666..8e3d5038fc07 100644 --- a/pkg/sql/opt/ops/README.md +++ b/pkg/sql/opt/ops/README.md @@ -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 diff --git a/pkg/sql/opt/ops/scalar.opt b/pkg/sql/opt/ops/scalar.opt index 8c605f878296..bc40d0ec8439 100644 --- a/pkg/sql/opt/ops/scalar.opt +++ b/pkg/sql/opt/ops/scalar.opt @@ -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 diff --git a/pkg/sql/opt/optbuilder/window.go b/pkg/sql/opt/optbuilder/window.go index 8fd945938835..ce2c14e85f0b 100644 --- a/pkg/sql/opt/optbuilder/window.go +++ b/pkg/sql/opt/optbuilder/window.go @@ -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 } diff --git a/pkg/sql/opt/optgen/cmd/optgen/metadata.go b/pkg/sql/opt/optgen/cmd/optgen/metadata.go index ad53ad4e7451..229d7674d130 100644 --- a/pkg/sql/opt/optgen/cmd/optgen/metadata.go +++ b/pkg/sql/opt/optgen/cmd/optgen/metadata.go @@ -406,11 +406,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++ diff --git a/pkg/sql/opt/optgen/cmd/optgen/validator.go b/pkg/sql/opt/optgen/cmd/optgen/validator.go index a402e8acc732..ebd6ef872757 100644 --- a/pkg/sql/opt/optgen/cmd/optgen/validator.go +++ b/pkg/sql/opt/optgen/cmd/optgen/validator.go @@ -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 } } diff --git a/pkg/sql/opt/optgen/exprgen/testdata/limit b/pkg/sql/opt/optgen/exprgen/testdata/limit index af754f21d2cb..46121b7e8933 100644 --- a/pkg/sql/opt/optgen/exprgen/testdata/limit +++ b/pkg/sql/opt/optgen/exprgen/testdata/limit @@ -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") ) ---- @@ -30,7 +30,7 @@ limit expr (Limit (Sort (Scan [ (Table "abc") (Cols "a,b") ])) - (Const 10) + (Const 10 "int") (OrderingChoice "+a") ) ---- diff --git a/pkg/sql/opt/optgen/exprgen/testdata/scalar b/pkg/sql/opt/optgen/exprgen/testdata/scalar index 83a2950481fb..bfab56e9318b 100644 --- a/pkg/sql/opt/optgen/exprgen/testdata/scalar +++ b/pkg/sql/opt/optgen/exprgen/testdata/scalar @@ -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] diff --git a/pkg/sql/opt/optgen/exprgen/testdata/scan b/pkg/sql/opt/optgen/exprgen/testdata/scan index bbf22b54cb61..d42b6e7b324a 100644 --- a/pkg/sql/opt/optgen/exprgen/testdata/scan +++ b/pkg/sql/opt/optgen/exprgen/testdata/scan @@ -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 diff --git a/pkg/sql/opt/optgen/exprgen/testdata/values b/pkg/sql/opt/optgen/exprgen/testdata/values index 2e9997dd57c8..dbb56ff93c9b 100644 --- a/pkg/sql/opt/optgen/exprgen/testdata/values +++ b/pkg/sql/opt/optgen/exprgen/testdata/values @@ -1,8 +1,8 @@ expr (Values [ - (Tuple [ (Const 1) (Const 1) ] "tuple{int, int}" ) - (Tuple [ (Const 2) (Const 2) ] "tuple{int, int}" ) + (Tuple [ (Const 1 "int") (Const 1 "int") ] "tuple{int, int}" ) + (Tuple [ (Const 2 "int") (Const 2 "int") ] "tuple{int, int}" ) ] [ (Cols [ (NewColumn "a" "int") (NewColumn "b" "int") ]) ] ) @@ -23,10 +23,10 @@ values expr (Project (Values - [ (Tuple [ (Const 1) ] "tuple{int}" ) ] + [ (Tuple [ (Const 1 "int") ] "tuple{int}" ) ] [ (Cols [ (NewColumn "x" "int") ]) ] ) - [ (ProjectionsItem (Plus (Var "x") (Const 10)) (NewColumn "y" "int")) ] + [ (ProjectionsItem (Plus (Var "x") (Const 10 "int")) (NewColumn "y" "int")) ] "x" ) ---- diff --git a/pkg/sql/opt/xform/custom_funcs.go b/pkg/sql/opt/xform/custom_funcs.go index dcc6cc76be25..3ee36f17bc69 100644 --- a/pkg/sql/opt/xform/custom_funcs.go +++ b/pkg/sql/opt/xform/custom_funcs.go @@ -1397,11 +1397,13 @@ func (c *CustomFuncs) GenerateLookupJoins( if !ok { break } + idxColType := c.e.f.Metadata().ColumnMeta(idxCol).Type constColID := c.e.f.Metadata().AddColumn( fmt.Sprintf("project_const_col_@%d", idxCol), - condition.Right.DataType()) + idxColType, + ) projections = append(projections, c.e.f.ConstructProjectionsItem( - c.e.f.ConstructConst(constValMap[idxCol]), + c.e.f.ConstructConst(constValMap[idxCol], condition.Right.DataType()), constColID, )) diff --git a/pkg/sql/opt/xform/optimizer_test.go b/pkg/sql/opt/xform/optimizer_test.go index c7efa663e6fe..7205e32755be 100644 --- a/pkg/sql/opt/xform/optimizer_test.go +++ b/pkg/sql/opt/xform/optimizer_test.go @@ -25,6 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/testcat" "github.com/cockroachdb/cockroach/pkg/sql/opt/xform" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/datadriven" ) @@ -109,7 +110,7 @@ func TestDetachMemoRace(t *testing.T) { memo.FiltersExpr{f.ConstructFiltersItem( f.ConstructEq( f.ConstructVariable(col), - f.ConstructConst(tree.NewDInt(10)), + f.ConstructConst(tree.NewDInt(10), types.Int), ), )}, ) diff --git a/pkg/sql/opt/xform/rules/groupby.opt b/pkg/sql/opt/xform/rules/groupby.opt index 0ee0012dee93..19414a2af676 100644 --- a/pkg/sql/opt/xform/rules/groupby.opt +++ b/pkg/sql/opt/xform/rules/groupby.opt @@ -23,7 +23,7 @@ $input [ (FiltersItem (IsNot $variable (Null (AnyType)))) ] ) - (Const 1) + (IntConst 1) (MakeOrderingChoiceFromColumn (OpName $agg) $col) ) [ (AggregationsItem (ConstAgg $variable) $aggPrivate) ] @@ -68,7 +68,7 @@ (MakeProjectFromPassthroughAggs (Limit $input - (Const 1) + (IntConst 1) (MakeOrderingChoiceFromColumn Min $col) ) $aggregations @@ -96,7 +96,7 @@ (MakeProjectFromPassthroughAggs (Limit $input - (Const 1) + (IntConst 1) (MakeOrderingChoiceFromColumn Max $col) ) $aggregations