Commit 733b32eb authored by craig[bot]'s avatar craig[bot]

Merge #50678

50678: sql: remove Impure flag r=RaduBerinde a=RaduBerinde

This set of commits changes all uses of Impure to use volatilities instead, and removes the flag altogether.

#### sql: use volatility when sanitizing expressions

As part of work toward removing the coarse (and unreliable) `Impure` flag, this
change modifies the expression sanitizers to take a `maxVolatility` argument
instead of an `allowImpure` boolean. Currently we only support either Stable or
Volatile as max-volatility. There are some TODO cases where we may want to
support Immutable (but note that we would have to check operators and casts as
well).

Release note: None

#### sql: use volatility in normalization / IsConst

Release note: None

#### sqlsmith: use volatility in sqlsmith check

Update the impure check in sqlsmith to check the Volatility. We want only
immutable functions so the results of any given query are consistent across
servers.

Release note: None

#### sql: remove Impure flag

Release note: None
Co-authored-by: default avatarRadu Berinde <[email protected]>
parents e4b3bb90 a8434379
......@@ -1350,8 +1350,9 @@ INSERT INTO d.t2 VALUES (ARRAY['howdy']), (ARRAY['hi']);
CREATE TABLE d.expr (
x d.greeting,
y d.greeting DEFAULT 'hello',
z d.greeting[] AS (enum_range(x, 'hi')) STORED,
CHECK (x < 'hi')
z bool AS (y = 'howdy') STORED,
CHECK (x < 'hi'),
CHECK (x = ANY enum_range(y, 'hi'))
);
`)
// Now backup the database.
......@@ -1367,7 +1368,7 @@ CREATE TABLE d.expr (
// Insert a row into the expr table so that all of the expressions are
// evaluated and checked.
sqlDB.Exec(t, `INSERT INTO d.expr VALUES ('howdy')`)
sqlDB.CheckQueryResults(t, `SELECT * FROM d.expr`, [][]string{{"howdy", "hello", "{howdy,hi}"}})
sqlDB.CheckQueryResults(t, `SELECT * FROM d.expr`, [][]string{{"howdy", "hello", "false"}})
sqlDB.ExpectErr(t, `pq: failed to satisfy CHECK constraint`, `INSERT INTO d.expr VALUES ('hi')`)
// We should be able to use the restored types to create new tables.
......
......@@ -81,7 +81,7 @@ func parseValues(tableDesc *sqlbase.TableDescriptor, values string) ([]sqlbase.E
for colIdx, expr := range rowTuple {
col := &tableDesc.Columns[colIdx]
typedExpr, err := sqlbase.SanitizeVarFreeExpr(
ctx, expr, col.Type, "avro", &semaCtx, false /* allowImpure */)
ctx, expr, col.Type, "avro", &semaCtx, tree.VolatilityStable)
if err != nil {
return nil, err
}
......
......@@ -369,16 +369,21 @@ CREATE TABLE t (a INT, b INT, c INT, PRIMARY KEY (a, b)) PARTITION BY LIST (a) (
PARTITION p1 VALUES IN ((SELECT 1))
)
statement error PARTITION p1: now\(\): impure functions are not allowed in partition
statement error PARTITION p1: now\(\): context-dependent functions are not allowed in partition
CREATE TABLE t (a TIMESTAMP PRIMARY KEY) PARTITION BY LIST (a) (
PARTITION p1 VALUES IN (now())
)
statement error PARTITION p1: uuid_v4\(\): impure functions are not allowed in partition
statement error PARTITION p1: uuid_v4\(\): volatile functions are not allowed in partition
CREATE TABLE t (a TIMESTAMP PRIMARY KEY) PARTITION BY LIST (a) (
PARTITION p1 VALUES IN (uuid_v4() || 'foo')
)
statement error PARTITION p1: 'now'::TIMESTAMP: partition values must be constant
CREATE TABLE t (a TIMESTAMP PRIMARY KEY) PARTITION BY LIST (a) (
PARTITION p1 VALUES IN ('now')
)
statement ok
CREATE TABLE ok1 (
a INT, b INT, c INT,
......
......@@ -93,7 +93,9 @@ func valueEncodePartitionTuple(
var semaCtx tree.SemaContext
typedExpr, err := sqlbase.SanitizeVarFreeExpr(evalCtx.Context, expr, cols[i].Type, "partition",
&semaCtx, false /* allowImpure */)
&semaCtx,
tree.VolatilityImmutable,
)
if err != nil {
return nil, err
}
......
......@@ -341,7 +341,7 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx
return nil, false
}
fn := fns[s.rnd.Intn(len(fns))]
if s.disableImpureFns && fn.def.Impure {
if s.disableImpureFns && fn.overload.Volatility > tree.VolatilityImmutable {
return nil, false
}
for _, ignore := range s.ignoreFNs {
......
......@@ -245,7 +245,7 @@ func alterColumnTypeGeneral(
toType,
"ALTER COLUMN TYPE USING EXPRESSION",
&params.p.semaCtx,
true, /* allowImpure */
tree.VolatilityVolatile,
tn,
)
......
......@@ -826,7 +826,7 @@ func applyColumnMutation(
} else {
colDatumType := col.Type
expr, err := sqlbase.SanitizeVarFreeExpr(
params.ctx, t.Default, colDatumType, "DEFAULT", &params.p.semaCtx, true, /* allowImpure */
params.ctx, t.Default, colDatumType, "DEFAULT", &params.p.semaCtx, tree.VolatilityVolatile,
)
if err != nil {
return err
......
......@@ -141,6 +141,11 @@ func verifyColOperator(args verifyColOperatorArgs) error {
StreamingMemAccount: &acc,
DiskQueueCfg: colcontainer.DiskQueueCfg{FS: tempFS},
FDSemaphore: colexecbase.NewTestingSemaphore(256),
// TODO(yuzefovich): adjust expression generator to not produce
// mixed-type timestamp-related expressions and then disallow the
// fallback again.
ProcessorConstructor: rowexec.NewProcessor,
}
var spilled bool
if args.forceDiskSpill {
......
......@@ -48,8 +48,8 @@ func fillInPlaceholders(
return nil, errors.AssertionFailedf("no type for placeholder %s", idx)
}
typedExpr, err := sqlbase.SanitizeVarFreeExpr(
ctx, e, typ, "EXECUTE parameter", /* context */
&semaCtx, true /* allowImpure */)
ctx, e, typ, "EXECUTE parameter" /* context */, &semaCtx, tree.VolatilityVolatile,
)
if err != nil {
return nil, pgerror.WithCandidateCode(err, pgcode.WrongObjectType)
}
......
......@@ -289,14 +289,14 @@ CREATE TABLE y (
b INT AS (a) STORED
)
statement error impure functions are not allowed in computed column
statement error context-dependent functions are not allowed in computed column
CREATE TABLE y (
a TIMESTAMP AS (now()) STORED
)
statement error impure functions are not allowed in computed column
statement error volatile functions are not allowed in computed column
CREATE TABLE y (
a STRING AS (concat(now()::STRING, uuid_v4()::STRING)) STORED
a STRING AS (concat('foo', uuid_v4()::STRING)) STORED
)
statement error computed columns cannot reference other computed columns
......
......@@ -26,9 +26,12 @@ CREATE TABLE error (a INT, INDEX (a) WHERE b = 3)
# Don't allow mutable functions.
# TODO(mgartner): The error code for this should be 42P17, not 0A000.
statement error pgcode 0A000 impure functions are not allowed in index predicate
statement error pgcode 0A000 context-dependent functions are not allowed in index predicate
CREATE TABLE error (t TIMESTAMPTZ, INDEX (t) WHERE t < now())
statement error pgcode 0A000 volatile functions are not allowed in index predicate
CREATE TABLE error (t FLOAT, INDEX (t) WHERE t < random())
# Don't allow variable subexpressions.
statement error pgcode 42601 variable sub-expressions are not allowed in index predicate
CREATE TABLE error (a INT, INDEX (a) WHERE count(*) = 1)
......
......@@ -512,7 +512,7 @@ func (h *harness) prepareUsingAPI(tb testing.TB) {
typ,
"", /* context */
&h.semaCtx,
true, /* allowImpure */
tree.VolatilityVolatile,
)
if err != nil {
tb.Fatalf("%v", err)
......
......@@ -87,12 +87,12 @@ SELECT * FROM (SELECT concat(s, y::string) FROM a) AS q(v) WHERE v = 'foo'
----
select
├── columns: v:5(string!null)
├── stable
├── immutable
├── stats: [rows=20, distinct(5)=1, null(5)=0]
├── fd: ()-->(5)
├── project
│ ├── columns: concat:5(string)
│ ├── stable
│ ├── immutable
│ ├── stats: [rows=2000, distinct(5)=100, null(5)=0]
│ ├── scan a
│ │ ├── columns: x:1(int!null) y:2(int) s:3(string) d:4(decimal!null)
......@@ -100,7 +100,7 @@ select
│ │ ├── key: (1)
│ │ └── fd: (1)-->(2-4), (3,4)~~>(1,2)
│ └── projections
│ └── concat(s:3, y:2::STRING) [as=concat:5, type=string, outer=(2,3), stable]
│ └── concat(s:3, y:2::STRING) [as=concat:5, type=string, outer=(2,3), immutable]
└── filters
└── concat:5 = 'foo' [type=bool, outer=(5), constraints=(/5: [/'foo' - /'foo']; tight), fd=()-->(5)]
......@@ -111,13 +111,13 @@ SELECT * FROM (SELECT concat(s, y::string), x FROM a) AS q(v, x) GROUP BY v, x
group-by
├── columns: v:5(string) x:1(int!null)
├── grouping columns: x:1(int!null) concat:5(string)
├── stable
├── immutable
├── stats: [rows=2000, distinct(1,5)=2000, null(1,5)=0]
├── key: (1)
├── fd: (1)-->(5)
└── project
├── columns: concat:5(string) x:1(int!null)
├── stable
├── immutable
├── stats: [rows=2000, distinct(1,5)=2000, null(1,5)=0]
├── key: (1)
├── fd: (1)-->(5)
......@@ -127,7 +127,7 @@ group-by
│ ├── key: (1)
│ └── fd: (1)-->(2-4), (3,4)~~>(1,2)
└── projections
└── concat(s:3, y:2::STRING) [as=concat:5, type=string, outer=(2,3), stable]
└── concat(s:3, y:2::STRING) [as=concat:5, type=string, outer=(2,3), immutable]
# No available stats for column y.
build
......@@ -250,12 +250,12 @@ SELECT * FROM (SELECT concat(s, y::string) FROM a) AS q(v) WHERE v = 'foo'
----
select
├── columns: v:5(string!null)
├── stable
├── immutable
├── stats: [rows=1, distinct(5)=1, null(5)=0]
├── fd: ()-->(5)
├── project
│ ├── columns: concat:5(string)
│ ├── stable
│ ├── immutable
│ ├── stats: [rows=2000, distinct(5)=2000, null(5)=0]
│ ├── scan a
│ │ ├── columns: x:1(int!null) y:2(int) s:3(string) d:4(decimal!null)
......@@ -263,7 +263,7 @@ select
│ │ ├── key: (1)
│ │ └── fd: (1)-->(2-4), (3,4)~~>(1,2)
│ └── projections
│ └── concat(s:3, y:2::STRING) [as=concat:5, type=string, outer=(2,3), stable]
│ └── concat(s:3, y:2::STRING) [as=concat:5, type=string, outer=(2,3), immutable]
└── filters
└── concat:5 = 'foo' [type=bool, outer=(5), constraints=(/5: [/'foo' - /'foo']; tight), fd=()-->(5)]
......
......@@ -95,7 +95,7 @@ func (b *CheckConstraintBuilder) Build(
types.Bool,
"CHECK",
b.semaCtx,
true, /* allowImpure */
tree.VolatilityVolatile,
&b.tableName,
)
if err != nil {
......
......@@ -123,7 +123,7 @@ func (v *ComputedColumnValidator) Validate(d *tree.ColumnTableDef) error {
defType,
"computed column",
v.semaCtx,
false, /* allowImpure */
tree.VolatilityImmutable,
v.tableName,
)
if err != nil {
......
......@@ -60,7 +60,7 @@ func DequalifyAndValidateExpr(
typ *types.T,
op string,
semaCtx *tree.SemaContext,
allowImpure bool,
maxVolatility tree.Volatility,
tn *tree.TableName,
) (tree.TypedExpr, sqlbase.TableColSet, error) {
var colIDs sqlbase.TableColSet
......@@ -88,7 +88,7 @@ func DequalifyAndValidateExpr(
typ,
op,
semaCtx,
allowImpure,
maxVolatility,
)
if err != nil {
......
......@@ -42,27 +42,31 @@ func TestValidateExpr(t *testing.T) {
expectedValid bool
expectedExpr string
typ *types.T
allowImpure bool
maxVolatility tree.Volatility
}{
// De-qualify column names.
{"bar.a", true, "a", types.Bool, false},
{"foo.bar.a", true, "a", types.Bool, false},
{"bar.b = 0", true, "b = 0:::INT8", types.Bool, false},
{"foo.bar.b = 0", true, "b = 0:::INT8", types.Bool, false},
{"bar.a AND foo.bar.b = 0", true, "a AND (b = 0:::INT8)", types.Bool, false},
{"bar.a", true, "a", types.Bool, tree.VolatilityImmutable},
{"foo.bar.a", true, "a", types.Bool, tree.VolatilityImmutable},
{"bar.b = 0", true, "b = 0:::INT8", types.Bool, tree.VolatilityImmutable},
{"foo.bar.b = 0", true, "b = 0:::INT8", types.Bool, tree.VolatilityImmutable},
{"bar.a AND foo.bar.b = 0", true, "a AND (b = 0:::INT8)", types.Bool, tree.VolatilityImmutable},
// Validates the type of the expression.
{"concat(c, c)", true, "concat(c, c)", types.String, false},
{"concat(c, c)", false, "", types.Int, false},
{"b + 1", true, "b + 1:::INT8", types.Int, false},
{"b + 1", false, "", types.Bool, false},
{"concat(c, c)", true, "concat(c, c)", types.String, tree.VolatilityImmutable},
{"concat(c, c)", false, "", types.Int, tree.VolatilityImmutable},
{"b + 1", true, "b + 1:::INT8", types.Int, tree.VolatilityImmutable},
{"b + 1", false, "", types.Bool, tree.VolatilityImmutable},
// Validates that the expression has no variable expressions.
{"$1", false, "", types.Any, false},
// Validates that impure functions are allowed or disallowed.
{"now()", true, "now():::TIMESTAMPTZ", types.TimestampTZ, true},
{"now()", false, "", types.Any, false},
{"$1", false, "", types.Any, tree.VolatilityImmutable},
// Validates the volatility check.
{"now()", true, "now():::TIMESTAMPTZ", types.TimestampTZ, tree.VolatilityVolatile},
{"now()", true, "now():::TIMESTAMPTZ", types.TimestampTZ, tree.VolatilityStable},
{"now()", false, "", types.Any, tree.VolatilityImmutable},
{"uuid_v4()::STRING", true, "uuid_v4()::STRING", types.String, tree.VolatilityVolatile},
{"uuid_v4()::STRING", false, "", types.String, tree.VolatilityStable},
{"uuid_v4()::STRING", false, "", types.String, tree.VolatilityImmutable},
}
for _, d := range testData {
......@@ -79,7 +83,7 @@ func TestValidateExpr(t *testing.T) {
d.typ,
"test-validate-expr",
&semaCtx,
d.allowImpure,
d.maxVolatility,
&tn,
)
......
......@@ -52,7 +52,7 @@ func NewIndexPredicateValidator(
// - It results in a boolean.
// - It refers only to columns in the table.
// - It does not include subqueries.
// - It does not include mutable, aggregate, window, or set returning
// - It does not include non-immutable, aggregate, window, or set returning
// functions.
//
func (v *IndexPredicateValidator) Validate(expr tree.Expr) (tree.Expr, error) {
......@@ -63,7 +63,7 @@ func (v *IndexPredicateValidator) Validate(expr tree.Expr) (tree.Expr, error) {
types.Bool,
"index predicate",
v.semaCtx,
false, /* allowImpure */
tree.VolatilityImmutable,
&v.tableName,
)
if err != nil {
......
......@@ -38,9 +38,6 @@ func initAggregateBuiltins() {
panic("duplicate builtin: " + k)
}
if !v.props.Impure {
panic(fmt.Sprintf("%s: aggregate functions should all be impure, found %v", k, v))
}
if v.props.Class != tree.AggregateClass {
panic(fmt.Sprintf("%s: aggregate functions should be marked with the tree.AggregateClass "+
"function class, found %v", k, v))
......@@ -61,7 +58,7 @@ func initAggregateBuiltins() {
}
func aggProps() tree.FunctionProperties {
return tree.FunctionProperties{Class: tree.AggregateClass, Impure: true}
return tree.FunctionProperties{Class: tree.AggregateClass}
}
func aggPropsNullableArgs() tree.FunctionProperties {
......
This diff is collapsed.
......@@ -43,9 +43,6 @@ func initGeneratorBuiltins() {
panic("duplicate builtin: " + k)
}
if !v.props.Impure {
panic(fmt.Sprintf("generator functions should all be impure, found %v", v))
}
if v.props.Class != tree.GeneratorClass {
panic(fmt.Sprintf("generator functions should be marked with the tree.GeneratorClass "+
"function class, found %v", v))
......@@ -57,7 +54,6 @@ func initGeneratorBuiltins() {
func genProps() tree.FunctionProperties {
return tree.FunctionProperties{
Impure: true,
Class: tree.GeneratorClass,
Category: categoryGenerator,
}
......@@ -65,7 +61,6 @@ func genProps() tree.FunctionProperties {
func genPropsWithLabels(returnLabels []string) tree.FunctionProperties {
return tree.FunctionProperties{
Impure: true,
Class: tree.GeneratorClass,
Category: categoryGenerator,
ReturnLabels: returnLabels,
......@@ -265,7 +260,6 @@ var generators = map[string]builtinDefinition{
"crdb_internal.check_consistency": makeBuiltin(
tree.FunctionProperties{
Impure: true,
Class: tree.GeneratorClass,
Category: categorySystemInfo,
},
......
......@@ -2911,7 +2911,6 @@ The calculations are done on a sphere.`,
tree.FunctionProperties{
Class: tree.SQLClass,
Category: categoryGeospatial,
Impure: true,
},
tree.Overload{
Types: tree.ArgTypes{
......
......@@ -1083,11 +1083,7 @@ SELECT description
),
"pg_sleep": makeBuiltin(
tree.FunctionProperties{
// pg_sleep is marked as impure so it doesn't get executed during
// normalization.
Impure: true,
},
tree.FunctionProperties{},
tree.Overload{
Types: tree.ArgTypes{{"seconds", types.Float}},
ReturnType: tree.FixedReturnType(types.Bool),
......@@ -1744,7 +1740,6 @@ SELECT description
tree.FunctionProperties{
Category: categorySystemInfo,
DistsqlBlocklist: true,
Impure: true,
},
tree.Overload{
Types: tree.ArgTypes{{"setting_name", types.String}, {"new_value", types.String}, {"is_local", types.Bool}},
......
......@@ -27,9 +27,6 @@ func initWindowBuiltins() {
panic("duplicate builtin: " + k)
}
if !v.props.Impure {
panic(fmt.Sprintf("%s: window functions should all be impure, found %v", k, v))
}
if v.props.Class != tree.WindowClass {
panic(fmt.Sprintf("%s: window functions should be marked with the tree.WindowClass "+
"function class, found %v", k, v))
......@@ -46,8 +43,7 @@ func initWindowBuiltins() {
func winProps() tree.FunctionProperties {
return tree.FunctionProperties{
Impure: true,
Class: tree.WindowClass,
Class: tree.WindowClass,
}
}
......
......@@ -30,7 +30,7 @@ import (
"github.com/stretchr/testify/require"
)
func prepareExpr(t *testing.T, datumExpr string) tree.TypedExpr {
func prepareExpr(t *testing.T, datumExpr string) tree.Datum {
expr, err := parser.ParseExpr(datumExpr)
if err != nil {
t.Fatalf("%s: %v", datumExpr, err)
......@@ -50,7 +50,11 @@ func prepareExpr(t *testing.T, datumExpr string) tree.TypedExpr {
if err != nil {
t.Fatalf("%s: %v", datumExpr, err)
}
return typedExpr
d, err := typedExpr.Eval(evalCtx)
if err != nil {
t.Fatalf("%s: %v", datumExpr, err)
}
return d
}
func TestDatumOrdering(t *testing.T) {
......@@ -229,9 +233,8 @@ func TestDatumOrdering(t *testing.T) {
}
ctx := tree.NewTestingEvalContext(cluster.MakeTestingClusterSettings())
for _, td := range testData {
expr := prepareExpr(t, td.datumExpr)
d := prepareExpr(t, td.datumExpr)
d := expr.(tree.Datum)
prevVal, hasPrev := d.Prev(ctx)
nextVal, hasNext := d.Next(ctx)
if td.prev == noPrev {
......
......@@ -1394,13 +1394,6 @@ func (node *FuncExpr) IsWindowFunctionApplication() bool {
return node.WindowDef != nil
}
// IsImpure returns whether the function application is impure, meaning that it
// potentially returns a different value when called in the same statement with
// the same parameters.
func (node *FuncExpr) IsImpure() bool {
return node.fnProps != nil && node.fnProps.Impure
}
// IsDistSQLBlocklist returns whether the function is not supported by DistSQL.
func (node *FuncExpr) IsDistSQLBlocklist() bool {
return node.fnProps != nil && node.fnProps.DistsqlBlocklist
......
......@@ -57,14 +57,6 @@ type FunctionProperties struct {
// be NULL and should act accordingly.
NullableArgs bool
// Impure is set to true when a function potentially returns a
// different value when called in the same statement with the same
// parameters. e.g.: random(), clock_timestamp(). Some functions
// like now() return the same value in the same statement, but
// different values in separate statements, and should not be marked
// as impure.
Impure bool
// DistsqlBlocklist is set to true when a function depends on
// members of the EvalContext that are not marshaled by DistSQL
// (e.g. planner). Currently used for DistSQL to determine if
......
......@@ -816,22 +816,37 @@ var _ Visitor = &isConstVisitor{}
func (v *isConstVisitor) VisitPre(expr Expr) (recurse bool, newExpr Expr) {
if v.isConst {
if isVar(v.ctx, expr, true /*allowConstPlaceholders*/) {
if !operatorIsImmutable(expr) || isVar(v.ctx, expr, true /*allowConstPlaceholders*/) {
v.isConst = false
return false, expr
}
switch t := expr.(type) {
case *FuncExpr:
if t.IsImpure() {
v.isConst = false
return false, expr
}
}
}
return true, expr
}
func operatorIsImmutable(expr Expr) bool {
switch t := expr.(type) {
case *FuncExpr:
return t.fnProps.Class == NormalClass && t.fn.Volatility <= VolatilityImmutable
case *CastExpr:
volatility, ok := LookupCastVolatility(t.Expr.(TypedExpr).ResolvedType(), t.typ)
return ok && volatility <= VolatilityImmutable
case *UnaryExpr:
return t.fn.Volatility <= VolatilityImmutable
case *BinaryExpr:
return t.Fn.Volatility <= VolatilityImmutable
case *ComparisonExpr:
return t.fn.Volatility <= VolatilityImmutable
default:
return true
}
}
func (*isConstVisitor) VisitPost(expr Expr) Expr { return expr }
func (v *isConstVisitor) run(expr Expr) bool {
......@@ -842,7 +857,7 @@ func (v *isConstVisitor) run(expr Expr) bool {
// IsConst returns whether the expression is constant. A constant expression
// does not contain variables, as defined by ContainsVars, nor impure functions.
func IsConst(evalCtx *EvalContext, expr Expr) bool {
func IsConst(evalCtx *EvalContext, expr TypedExpr) bool {
v := isConstVisitor{ctx: evalCtx}
return v.run(expr)
}
......@@ -885,22 +900,14 @@ func (v *fastIsConstVisitor) VisitPre(expr Expr) (recurse bool, newExpr Expr) {
}
v.visited = true
// If the parent expression is a variable or impure function, we know that it
// is not constant.
// If the parent expression is a variable or non-immutable operator, we know
// that it is not constant.
if isVar(v.ctx, expr, true /*allowConstPlaceholders*/) {
if !operatorIsImmutable(expr) || isVar(v.ctx, expr, true /*allowConstPlaceholders*/) {
v.isConst = false
return false, expr
}
switch t := expr.(type) {
case *FuncExpr:
if t.IsImpure() {
v.isConst = false
return false, expr
}
}
return true, expr
}
......
......@@ -134,8 +134,11 @@ const (
// This is used e.g. when processing the calls inside ROWS FROM.
RejectNestedGenerators
// RejectImpureFunctions rejects any non-const functions like now().
RejectImpureFunctions
// RejectStableFunctions rejects any stable functions.
RejectStableFunctions
// RejectVolatileFunctions rejects any volatile functions.
RejectVolatileFunctions
// RejectSubqueries rejects subqueries in scalar contexts.
RejectSubqueries
......@@ -161,10 +164,6 @@ type ScalarProperties struct {
// contained a SRF.
SeenGenerator bool
// SeenImpureFunctions is set to true if the expression originally
// contained an impure function.
SeenImpure bool
// inFuncExpr is temporarily set to true while type checking the
// parameters of a function. Used to process RejectNestedGenerators
// properly.
......@@ -824,15 +823,34 @@ func (sc *SemaContext) checkFunctionUsage(expr *FuncExpr, def *FunctionDefinitio
}
sc.Properties.Derived.SeenGenerator = true
}
if def.Impure {
if sc.Properties.required.rejectFlags&RejectImpureFunctions != 0 {
return nil
}
// checkOverloadUsage checks whether the given built-in overload is allowed in
// the current context.
func (sc *SemaContext) checkOverloadUsage(overload *Overload) error {
if sc == nil {
return nil
}
switch overload.Volatility {
case VolatilityVolatile:
if sc.Properties.required.rejectFlags&RejectVolatileFunctions != 0 {
// The code FeatureNotSupported is a bit misleading here,
// because we probably can't support the feature at all. However
// this error code matches PostgreSQL's in the same conditions.
return pgerror.Newf(pgcode.FeatureNotSupported,
"impure functions are not allowed in %s", sc.Properties.required.context)
"volatile functions are not allowed in %s", sc.Properties.required.context)
}
case VolatilityStable:
if sc.Properties.required.rejectFlags&RejectStableFunctions != 0 {
// The code FeatureNotSupported is a bit misleading here,
// because we probably can't support the feature at all. However
// this error code matches PostgreSQL's in the same conditions.
return pgerror.Newf(pgcode.FeatureNotSupported,
"context-dependent functions are not allowed in %s",
sc.Properties.required.context,
)
}
sc.Properties.Derived.SeenImpure = true
}
return nil
}
......@@ -891,8 +909,7 @@ func (expr *FuncExpr) TypeCheck(
typedSubExprs, fns, err := typeCheckOverloadedExprs(ctx, semaCtx, desired, def.Definition, false, expr.Exprs...)
if err != nil {
return nil, pgerror.Wrapf(err, pgcode.InvalidParameterValue,
"%s()", def.Name)
return nil, pgerror.Wrapf(err, pgcode.InvalidParameterValue, "%s()", def.Name)
}
// If the function is an aggregate that does not accept null arguments and we
......@@ -1020,6 +1037,9 @@ func (expr *FuncExpr) TypeCheck(
strings.Join(typeNames, ", "),
)
}
if err := semaCtx.checkOverloadUsage(overloadImpl); err != nil {
return nil, pgerror.Wrapf(err, pgcode.InvalidParameterValue, "%s()", def.Name)
}
if overloadImpl.counter != nil {
telemetry.Inc(overloadImpl.counter)
}
......
<
......@@ -38,7 +38,7 @@ func SanitizeVarFreeExpr(
expectedType *types.T,
context string,
semaCtx *tree.SemaContext,
allowImpure bool,
maxVolatility tree.Volatility,
) (tree.TypedExpr, error) {
if tree.ContainsVars(expr) {
return nil, pgerror.Newf(pgcode.Syntax,
......@@ -52,8 +52,22 @@ func SanitizeVarFreeExpr(
// Ensure that the expression doesn't contain special functions.
flags := tree.RejectSpecial
if !allowImpure {
flags |= tree.RejectImpureFunctions
switch maxVolatility {
case tree.VolatilityImmutable:
// TODO(radu): we only check the volatility of functions; we need to check
// the volatility of operators and casts as well!
flags |= tree.RejectStableFunctions
fallthrough
case tree.VolatilityStable: