Skip to content

Commit

Permalink
sql: move FunctionClass from FunctionProperties to Overload
Browse files Browse the repository at this point in the history
`FunctionProperties` are attached to a `FunctionDefinition`, which is
simply a collection of overloads that share the same name. Most of the
fields in `FunctionProperties` are, however, overload-specific. They
should be moved to the `Overload` struct. In the long-term,
the hierarchy of function definitions, each with child function overloads,
should be flattened to a just overloads.

This commit takes one step in this direction.

Epic: CRDB-20370

Release note: None
  • Loading branch information
mgartner committed Jan 27, 2023
1 parent a07742a commit acef311
Show file tree
Hide file tree
Showing 18 changed files with 152 additions and 182 deletions.
3 changes: 2 additions & 1 deletion pkg/ccl/changefeedccl/encoder_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ func init() {
}
return tree.NewDBytes(tree.DBytes(o)), nil
},
Info: "Strings can be of the form 'resolved' or 'resolved=1s'.",
Class: tree.NormalClass,
Info: "Strings can be of the form 'resolved' or 'resolved=1s'.",
// Probably actually stable, but since this is tightly coupled to changefeed logic by design,
// best to be defensive.
Volatility: volatility.Volatile,
Expand Down
1 change: 0 additions & 1 deletion pkg/ccl/utilccl/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
// RegisterCCLBuiltin adds a builtin defined in CCL code to the global builtins registry.
func RegisterCCLBuiltin(name string, description string, overload tree.Overload) {
props := tree.FunctionProperties{
Class: tree.NormalClass,
Category: `CCL-only internal function`,
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/docgen/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func generateFunctions(from []string, categorize bool) []byte {
}
// We generate docs for both aggregates and window functions in separate
// files, so we want to omit them when processing all builtins.
if categorize && (props.Class == tree.AggregateClass || props.Class == tree.WindowClass) {
if categorize && (fn.Class == tree.AggregateClass || fn.Class == tree.WindowClass) {
continue
}
args := fn.Types.String()
Expand Down
7 changes: 4 additions & 3 deletions pkg/internal/sqlsmith/scalar.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,19 +440,20 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx
args = append(args, castType(arg, argTyp))
}

if fn.def.Class == tree.WindowClass && s.disableWindowFuncs {
if fn.overload.Class == tree.WindowClass && s.disableWindowFuncs {
return nil, false
}

if fn.def.Class == tree.AggregateClass && s.disableAggregateFuncs {
if fn.overload.Class == tree.AggregateClass && s.disableAggregateFuncs {
return nil, false
}

var window *tree.WindowDef
// Use a window function if:
// - we chose an aggregate function, then 1/6 chance, but not if we're in a HAVING (noWindow == true)
// - we explicitly chose a window function
if fn.def.Class == tree.WindowClass || (!s.disableWindowFuncs && !ctx.noWindow && s.d6() == 1 && fn.def.Class == tree.AggregateClass) {
if fn.overload.Class == tree.WindowClass ||
(!s.disableWindowFuncs && !ctx.noWindow && s.d6() == 1 && fn.overload.Class == tree.AggregateClass) {
var parts tree.Exprs
s.sample(len(refs), 2, func(i int) {
parts = append(parts, refs[i].item)
Expand Down
8 changes: 4 additions & 4 deletions pkg/internal/sqlsmith/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,9 +519,6 @@ var functions = func() map[tree.FunctionClass]map[oid.Oid][]function {
if skip {
continue
}
if _, ok := m[def.Class]; !ok {
m[def.Class] = map[oid.Oid][]function{}
}
// Ignore pg compat functions since many are unimplemented.
if def.Category == "Compatibility" {
continue
Expand All @@ -530,6 +527,9 @@ var functions = func() map[tree.FunctionClass]map[oid.Oid][]function {
continue
}
for _, ov := range def.Definition {
if m[ov.Class] == nil {
m[ov.Class] = map[oid.Oid][]function{}
}
// Ignore documented unusable functions.
if strings.Contains(ov.Info, "Not usable") {
continue
Expand All @@ -544,7 +544,7 @@ var functions = func() map[tree.FunctionClass]map[oid.Oid][]function {
if !found {
continue
}
m[def.Class][typ.Oid()] = append(m[def.Class][typ.Oid()], function{
m[ov.Class][typ.Oid()] = append(m[ov.Class][typ.Oid()], function{
def: def,
overload: ov,
})
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/norm/fold_constants_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ func (c *CustomFuncs) FoldColumnAccess(
// See FoldFunctionWithNullArg for more details.
func (c *CustomFuncs) CanFoldFunctionWithNullArg(private *memo.FunctionPrivate) bool {
return !private.Overload.CalledOnNullInput &&
private.Properties.Class == tree.NormalClass
private.Overload.Class == tree.NormalClass
}

// HasNullArg returns true if one of args is Null.
Expand All @@ -647,7 +647,7 @@ func (c *CustomFuncs) FoldFunction(
) (_ opt.ScalarExpr, ok bool) {
// Non-normal function classes (aggregate, window, generator) cannot be
// folded into a single constant.
if private.Properties.Class != tree.NormalClass {
if private.Overload.Class != tree.NormalClass {
return nil, false
}

Expand Down
26 changes: 13 additions & 13 deletions pkg/sql/pg_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -2354,30 +2354,30 @@ https://www.postgresql.org/docs/9.6/view-pg-prepared-statements.html`,
}

func addPgProcBuiltinRow(name string, addRow func(...tree.Datum) error) error {
props, overloads := builtinsregistry.GetBuiltinProperties(name)
isAggregate := props.Class == tree.AggregateClass
isWindow := props.Class == tree.WindowClass
_, overloads := builtinsregistry.GetBuiltinProperties(name)
nspOid := tree.NewDOid(catconstants.PgCatalogID)
const crdbInternal = catconstants.CRDBInternalSchemaName + "."
if strings.HasPrefix(name, crdbInternal) {
nspOid = tree.NewDOid(catconstants.CrdbInternalID)
name = name[len(crdbInternal):]
}

var kind tree.Datum
switch {
case isAggregate:
kind = tree.NewDString("a")
case isWindow:
kind = tree.NewDString("w")
default:
kind = tree.NewDString("f")
}

for _, builtin := range overloads {
dName := tree.NewDName(name)
dSrc := tree.NewDString(name)

isAggregate := builtin.Class == tree.AggregateClass
isWindow := builtin.Class == tree.WindowClass
var kind tree.Datum
switch {
case isAggregate:
kind = tree.NewDString("a")
case isWindow:
kind = tree.NewDString("w")
default:
kind = tree.NewDString("f")
}

var retType tree.Datum
isRetSet := false
if fixedRetType := builtin.FixedReturnType(); fixedRetType != nil {
Expand Down
Loading

0 comments on commit acef311

Please sign in to comment.