Skip to content
Open
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
29 changes: 29 additions & 0 deletions pkg/sql/create_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,9 @@ func (n *createFunctionNode) replaceFunction(
if err := setFuncOptions(params, udfDesc, n.cf.Options); err != nil {
return err
}
if err := validateVolatilityForDependents(params.ctx, params.p, udfDesc); err != nil {
return err
}

// Removing all existing references before adding new references.
for _, id := range udfDesc.DependsOn {
Expand Down Expand Up @@ -673,6 +676,32 @@ func validateVolatilityInOptions(
return nil
}

// validateVolatilityForDependents rejects a volatility weakening when the
// function is referenced by an IMMUTABLE-requiring dependent (computed
// column, expression index, or CHECK constraint).
func validateVolatilityForDependents(
ctx context.Context, p *planner, udfDesc catalog.FunctionDescriptor,
) error {
if udfDesc.GetVolatility() == catpb.Function_IMMUTABLE {
return nil
}
for _, ref := range udfDesc.GetDependedOnBy() {
if len(ref.ColumnIDs) == 0 && len(ref.IndexIDs) == 0 && len(ref.ConstraintIDs) == 0 {
continue
}
depDesc, err := p.Descriptors().ByIDWithoutLeased(p.txn).WithoutNonPublic().Get().Table(ctx, ref.ID)
if err != nil {
return err
}
return pgerror.Newf(
pgcode.DependentObjectsStillExist,
"cannot change volatility of function %q because it is referenced by table %q in a context requiring IMMUTABLE",
udfDesc.GetName(), depDesc.GetName(),
)
}
return nil
}

// validateParameters checks that changes to the parameters, if any, are
// allowed. This method expects that the return type equality has already been
// checked.
Expand Down
67 changes: 67 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/udf_in_table
Original file line number Diff line number Diff line change
Expand Up @@ -882,3 +882,70 @@ CREATE FUNCTION f_circle() RETURNS INT LANGUAGE SQL AS $$ SELECT a FROM t_circle

statement error pgcode 42P13 pq: .*cannot add dependency from descriptor \d+ to function f_circle \(\d+\) because there will be a dependency cycle.*
ALTER TABLE t_circle ALTER COLUMN b SET DEFAULT f_circle();

# Verify that CREATE OR REPLACE FUNCTION refuses to weaken volatility when an
# existing dependent uses the function in a context requiring IMMUTABLE.

statement ok
CREATE FUNCTION f_vol() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ SELECT 1 $$

# Virtual computed column dependent.
statement ok
CREATE TABLE t_replace_vol_virt (a INT PRIMARY KEY, b INT AS (f_vol()) VIRTUAL)

statement error pgcode 2BP01 cannot change volatility of function "f_vol" because it is referenced by table "t_replace_vol_virt"
CREATE OR REPLACE FUNCTION f_vol() RETURNS INT LANGUAGE SQL VOLATILE AS $$ SELECT 2 $$

statement error pgcode 2BP01 cannot change volatility of function "f_vol" because it is referenced by table "t_replace_vol_virt"
CREATE OR REPLACE FUNCTION f_vol() RETURNS INT LANGUAGE SQL STABLE AS $$ SELECT 2 $$

# Omitting the volatility option implicitly defaults to VOLATILE.
statement error pgcode 2BP01 cannot change volatility of function "f_vol" because it is referenced by table "t_replace_vol_virt"
CREATE OR REPLACE FUNCTION f_vol() RETURNS INT LANGUAGE SQL AS $$ SELECT 2 $$

# Re-specifying IMMUTABLE preserves the invariant and is allowed.
statement ok
CREATE OR REPLACE FUNCTION f_vol() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ SELECT 3 $$

statement ok
DROP TABLE t_replace_vol_virt

# Stored computed column dependent.
statement ok
CREATE TABLE t_replace_vol_stored (a INT PRIMARY KEY, b INT AS (f_vol()) STORED)

statement error pgcode 2BP01 cannot change volatility of function "f_vol" because it is referenced by table "t_replace_vol_stored"
CREATE OR REPLACE FUNCTION f_vol() RETURNS INT LANGUAGE SQL VOLATILE AS $$ SELECT 2 $$

statement ok
DROP TABLE t_replace_vol_stored

# CHECK constraint dependent.
statement ok
CREATE TABLE t_replace_vol_check (a INT PRIMARY KEY, b INT, CHECK (b > f_vol()))

statement error pgcode 2BP01 cannot change volatility of function "f_vol" because it is referenced by table "t_replace_vol_check"
CREATE OR REPLACE FUNCTION f_vol() RETURNS INT LANGUAGE SQL VOLATILE AS $$ SELECT 2 $$

statement ok
DROP TABLE t_replace_vol_check

# Expression index dependent.
statement ok
CREATE TABLE t_replace_vol_idx (a INT PRIMARY KEY, b INT)

statement ok
CREATE INDEX idx_replace_vol ON t_replace_vol_idx ((f_vol() + b))

statement error pgcode 2BP01 cannot change volatility of function "f_vol" because it is referenced by table "t_replace_vol_idx"
CREATE OR REPLACE FUNCTION f_vol() RETURNS INT LANGUAGE SQL VOLATILE AS $$ SELECT 2 $$

statement ok
DROP TABLE t_replace_vol_idx

# Without IMMUTABLE-requiring dependents, the volatility can be freely changed.
statement ok
CREATE OR REPLACE FUNCTION f_vol() RETURNS INT LANGUAGE SQL VOLATILE AS $$ SELECT 2 $$

statement ok
DROP FUNCTION f_vol
9 changes: 8 additions & 1 deletion pkg/sql/opt/optbuilder/select.go
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,14 @@ func (b *Builder) buildScan(
// virtual columns.
proj := make(memo.ProjectionsExpr, 0, virtualColIDs.Len())
virtualColIDs.ForEach(func(col opt.ColumnID) {
item := b.factory.ConstructProjectionsItem(tabMeta.ComputedCols[col], col)
computedExpr := tabMeta.ComputedCols[col]
if computedExpr == nil {
panic(errors.WithHint(
errors.AssertionFailedf("virtual computed column has non-immutable expression"),
"a function referenced by this column may have been altered to non-IMMUTABLE volatility",
))
}
item := b.factory.ConstructProjectionsItem(computedExpr, col)
if !item.ScalarProps().OuterCols.SubsetOf(scanColIDs) {
panic(errors.AssertionFailedf("scanned virtual column depends on non-scanned column"))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlclustersettings"
"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
Expand Down Expand Up @@ -386,6 +387,8 @@ func replaceFunction(
}
}

validateVolatilityForDependents(b, fnID, n.Name.Object(), volatility)

// Always replace all option sub-elements, even if using defaults.
b.Replace(&scpb.FunctionVolatility{
FunctionID: fnID,
Expand Down Expand Up @@ -435,6 +438,70 @@ func replaceFunction(
b.LogEventForExistingTarget(existingFnElem)
}

// validateVolatilityForDependents rejects a volatility weakening when the
// function is referenced by an IMMUTABLE-requiring dependent (computed
// column, expression index, or CHECK constraint).
func validateVolatilityForDependents(
b BuildCtx, fnID catid.DescID, fnName string, newVolatility catpb.Function_Volatility,
) {
if newVolatility == catpb.Function_IMMUTABLE {
return
}
backRefs := b.BackReferences(fnID)
refsFunction := func(ids []catid.DescID) bool {
for _, id := range ids {
if id == fnID {
return true
}
}
return false
}
var depDescID catid.DescID
backRefs.FilterColumnComputeExpression().ForEach(
func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.ColumnComputeExpression) {
if depDescID == 0 && refsFunction(e.UsesFunctionIDs) {
depDescID = e.TableID
}
},
)
if depDescID == 0 {
backRefs.FilterCheckConstraint().ForEach(
func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.CheckConstraint) {
if depDescID == 0 && refsFunction(e.UsesFunctionIDs) {
depDescID = e.TableID
}
},
)
}
if depDescID == 0 {
backRefs.FilterCheckConstraintUnvalidated().ForEach(
func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.CheckConstraintUnvalidated) {
if depDescID == 0 && refsFunction(e.UsesFunctionIDs) {
depDescID = e.TableID
}
},
)
}
if depDescID == 0 {
backRefs.FilterSecondaryIndex().ForEach(
func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.SecondaryIndex) {
if depDescID == 0 && e.EmbeddedExpr != nil && refsFunction(e.EmbeddedExpr.UsesFunctionIDs) {
depDescID = e.TableID
}
},
)
}
if depDescID == 0 {
return
}
ns := b.QueryByID(depDescID).FilterNamespace().MustGetOneElement()
panic(pgerror.Newf(
pgcode.DependentObjectsStillExist,
"cannot change volatility of function %q because it is referenced by table %q in a context requiring IMMUTABLE",
fnName, ns.Name,
))
}

// updateDependentTriggers finds all triggers that reference the given function
// and updates their inlined function body and dependency tracking to reflect the
// new function body.
Expand Down
Loading