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

udf: name resolution broken when switching databases #93321

Closed
rafiss opened this issue Dec 9, 2022 · 3 comments · Fixed by #96045
Closed

udf: name resolution broken when switching databases #93321

rafiss opened this issue Dec 9, 2022 · 3 comments · Fixed by #96045
Assignees
Labels
A-sql-name-resolution SQL name resolution rules and CTEs. A-sql-routine UDFs and Stored Procedures C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-queries SQL Queries Team

Comments

@rafiss
Copy link
Collaborator

rafiss commented Dec 9, 2022

Describe the problem

If you resolve a UDF, then USE a different database, you can still resolve the old UDF by name even if it should no longer be visible.

To Reproduce

root@localhost:26257/defaultdb> create database d;
CREATE DATABASE

root@localhost:26257/defaultdb> select f(1);
ERROR: unknown function: f(): function undefined
SQLSTATE: 42883

root@localhost:26257/defaultdb> use d;
SET

root@localhost:26257/d> CREATE FUNCTION f(i INT) RETURNS INT LANGUAGE SQL AS 'SELECT i';
CREATE FUNCTION

root@localhost:26257/d> select f(1);
  f
-----
  1
(1 row)

root@localhost:26257/d> use defaultdb;
SET

root@localhost:26257/defaultdb> select f(1);
  f
-----
  1
(1 row)

Expected behavior
After switching databases, the name shouldn't be resolvable.

Environment:

  • CockroachDB version v22.2.0

Jira issue: CRDB-22274

@rafiss rafiss added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-name-resolution SQL name resolution rules and CTEs. A-sql-routine UDFs and Stored Procedures labels Dec 9, 2022
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Dec 9, 2022
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Dec 9, 2022
@ajwerner
Copy link
Contributor

ajwerner commented Dec 9, 2022

I think what's going on is the this code does not check on the UDFs.

// CheckDependencies resolves (again) each data source on which this metadata
// depends, in order to check that all data source names resolve to the same
// objects, and that the user still has sufficient privileges to access the
// objects. If the dependencies are no longer up-to-date, then CheckDependencies
// returns false.
//
// This function cannot swallow errors and return only a boolean, as it may
// perform KV operations on behalf of the transaction associated with the
// provided catalog, and those errors are required to be propagated.
func (md *Metadata) CheckDependencies(

@chengxiong-ruan
Copy link
Contributor

looks like same thing as #93082

@mgartner
Copy link
Collaborator

mgartner commented Dec 9, 2022

Thanks for tracking this down. Working on a fix now.

@mgartner mgartner self-assigned this Dec 9, 2022
@rafiss rafiss removed the T-sql-schema-deprecated Use T-sql-foundations instead label Dec 28, 2022
@mgartner mgartner assigned DrewKimball and unassigned DrewKimball and mgartner Jan 3, 2023
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Jan 31, 2023
This patch ensures that the metadata dependency-checking tracks user-defined
functions. This ensures that a cached query with a UDF reference will be
invalidated when the UDF is altered or dropped, or when the database is
switched.

Fixes cockroachdb#93082
Fixes cockroachdb#93321

Release note (bug fix): The query cache now checks to ensure that
user-defined functions referenced in the query have been altered or
dropped. This prevents a bug that could cause a query to return the
same result even after a UDF was dropped or the database was switched.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Feb 2, 2023
This patch ensures that the metadata dependency-checking tracks user-defined
functions. This ensures that a cached query with a UDF reference will be
invalidated when the UDF is altered or dropped, or when the database is
switched.

Fixes cockroachdb#93082
Fixes cockroachdb#93321

Release note (bug fix): The query cache now checks to ensure that
user-defined functions referenced in the query have been altered or
dropped. This prevents a bug that could cause a query to return the
same result even after a UDF was dropped or the database was switched.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Feb 22, 2023
This patch ensures that the metadata dependency-checking tracks user-defined
functions. This ensures that a cached query with a UDF reference will be
invalidated when the UDF is altered or dropped, or when the database is
switched.

Fixes cockroachdb#93082
Fixes cockroachdb#93321

Release note (bug fix): The query cache now checks to ensure that
user-defined functions referenced in the query have been altered or
dropped. This prevents a bug that could cause a query to return the
same result even after a UDF was dropped or the database was switched.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Feb 27, 2023
This patch ensures that the metadata dependency-checking tracks user-defined
functions. This ensures that a cached query with a UDF reference will be
invalidated when the UDF is altered or dropped, or when the database is
switched.

Fixes cockroachdb#93082
Fixes cockroachdb#93321

Release note (bug fix): The query cache now checks to ensure that
user-defined functions referenced in the query have been altered or
dropped. This prevents a bug that could cause a query to return the
same result even after a UDF was dropped or the database was switched.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Feb 28, 2023
This patch ensures that the metadata dependency-checking tracks user-defined
functions. This ensures that a cached query with a UDF reference will be
invalidated when the UDF is altered or dropped, or when the database is
switched.

Fixes cockroachdb#93082
Fixes cockroachdb#93321

Release note (bug fix): The query cache now checks to ensure that
user-defined functions referenced in the query have been altered or
dropped. This prevents a bug that could cause a query to return the
same result even after a UDF was dropped or the database was switched.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Mar 7, 2023
This patch adds tracking for user-defined functions in `opt.Metadata`.
This ensures that the query cache will be correctly invalidated after
a schema change or search-path change that could change the query's
semantics, or cause it to error.

Fixes cockroachdb#93082
Fixes cockroachdb#93321
Fixes cockroachdb#97757

Release note (bug fix): Fixed a bug that could prevent a cached query
from being invalidated when a UDF referenced by that query was altered
or dropped.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Mar 8, 2023
This patch adds tracking for user-defined functions in `opt.Metadata`.
This ensures that the query cache will be correctly invalidated after
a schema change or search-path change that could change the query's
semantics, or cause it to error.

Fixes cockroachdb#93082
Fixes cockroachdb#93321
Fixes cockroachdb#97757

Release note (bug fix): Fixed a bug that could prevent a cached query
from being invalidated when a UDF referenced by that query was altered
or dropped.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Mar 10, 2023
This patch adds tracking for user-defined functions in `opt.Metadata`.
This ensures that the query cache will be correctly invalidated after
a schema change or search-path change that could change the query's
semantics, or cause it to error.

Fixes cockroachdb#93082
Fixes cockroachdb#93321

Release note (bug fix): Fixed a bug that could prevent a cached query
from being invalidated when a UDF referenced by that query was altered
or dropped.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Mar 10, 2023
This patch adds tracking for user-defined functions in `opt.Metadata`.
This ensures that the query cache will be correctly invalidated after
a schema change or search-path change that could change the query's
semantics, or cause it to error.

Fixes cockroachdb#93082
Fixes cockroachdb#93321

Release note (bug fix): Fixed a bug that could prevent a cached query
from being invalidated when a UDF referenced by that query was altered
or dropped.
craig bot pushed a commit that referenced this issue Mar 20, 2023
96045: opt: check UDFs and UDTs by name when checking metadata dependencies r=DrewKimball a=DrewKimball

#### tree: distinguish UDFs and builtins that use a SQL string body

This refactor changes the meaning of the `Overload.IsUDF` field to
be true only for user-defined functions - meaning those created using
`CREATE FUNCTION`. This is contrasted with builtin functions that are
defined with a SQL string set in `Overload.Body`. Logic that cares only
about user-defined functions can continue checking `Overload.IsUDF`,
while cases that deal with the SQL body should use `Overload.HasSQLBody()`.
Note that it is insufficient to check whether `Overload.Body` is empty
because it is possible to define a UDF with an empty body.

#### opt: refactor metadata dependency tracking

This commit performs some refactoring for the way data source objects
are tracked in `opt.Metadata` in order to make future changes to UDT
and UDF dependency tracking easier. More particularly, UDTs and UDFs
will be able to re-resolve any references by name. This is necessary
in order to handle cases where changes to the search-path cause names
in the query to resolve to different objects.

#### opt: track UDT references by name in the Metadata

Previously, it was possible for invalid queries to be kept in the cache
after a schema change, since user-defined types were tracked only by OID.
This missed cases where the search path changed which object a name in
the query resolved to (or whether it resolved at all).

This patch fixes this behavior by tracking UDT references by name, similar
to what was already done for data sources. This ensures that the query
staleness check doesn't miss changes to the search path.

#### opt: track UDFs in the Metadata

This patch adds tracking for user-defined functions in `opt.Metadata`.
This ensures that the query cache will be correctly invalidated after
a schema change or search-path change that could change the query's
semantics, or cause it to error.

Fixes #95214
Fixes #93082
Fixes #93321
Fixes #96674

Release note (bug fix): Fixed a bug that could prevent a cached query from being invalidated when a UDF or UDT referenced by that query was altered or dropped, or when the schema or database of a UDF or UDT was altered or dropped.

98319: pkg/server: fix `/demologin` to properly redirect to home page r=dhartunian a=abarganier

With the introduction of the server controller, we introduced a layer between the HTTP handler and the HTTP server. When this was introduced, the logic to attempt a login to all tenants forgot to handle a specific case for `/demologin`, where the status code is set to a 302 redirect, instead of a 200 status OK.

This broke the redirect piece of the `/demologin` endpoint.

This patch updates the `attemptLoginToAllTenants` HTTP handler to properly set the 302 response code in the case where the underlying login function does so on the sessionWriter.

Release note: none

Epic: CRDB-12100

Fixes: #98253

98696: sql: disallow using cluster_logical_timestamp as column default when backfilling r=Xiang-Gu a=Xiang-Gu

Previously, `ADD COLUMN ... DEFAULT cluster_logical_timestamp()` would crash the node and leave the table in a corrupt state. The root cause is a nil pointer dereference. This commit fixed it by returning an unimplemented error and hence disallow using this builtin function as default value when backfilling.

Fixes: #98269
Release note (bug fix): fixed a bug as detailed in #98269.

Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
Co-authored-by: Alex Barganier <abarganier@cockroachlabs.com>
Co-authored-by: Xiang Gu <xiang@cockroachlabs.com>
@craig craig bot closed this as completed in 57dacce Mar 20, 2023
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Mar 30, 2023
This patch adds tracking for user-defined functions in `opt.Metadata`.
This ensures that the query cache will be correctly invalidated after
a schema change or search-path change that could change the query's
semantics, or cause it to error.

Fixes cockroachdb#93082
Fixes cockroachdb#93321

Release note (bug fix): Fixed a bug that could prevent a cached query
from being invalidated when a UDF referenced by that query was altered
or dropped.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Mar 31, 2023
This patch adds tracking for user-defined functions in `opt.Metadata`.
This ensures that the query cache will be correctly invalidated after
a schema change or search-path change that could change the query's
semantics, or cause it to error.

Fixes cockroachdb#93082
Fixes cockroachdb#93321

Release note (bug fix): Fixed a bug that could prevent a cached query
from being invalidated when a UDF referenced by that query was altered
or dropped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-name-resolution SQL name resolution rules and CTEs. A-sql-routine UDFs and Stored Procedures C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants