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

sql: allow * expressions in user-defined function (UDF) bodies #90080

Closed
mgartner opened this issue Oct 17, 2022 · 2 comments
Closed

sql: allow * expressions in user-defined function (UDF) bodies #90080

mgartner opened this issue Oct 17, 2022 · 2 comments
Assignees
Labels
A-sql-routine UDFs and Stored Procedures C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) docs-known-limitation T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@mgartner
Copy link
Collaborator

mgartner commented Oct 17, 2022

In v22.2 we disallow * expressions in UDF bodies, see #86070. We should allow these expressions. We'll need to figure out how to handle cases where a table's schema changes after a function has been created.

Jira issue: CRDB-20579

Epic CRDB-19496

@mgartner mgartner added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Oct 17, 2022
@blathers-crl blathers-crl bot added T-sql-queries SQL Queries Team T-sql-schema-deprecated Use T-sql-foundations instead labels Oct 17, 2022
mgartner added a commit to mgartner/cockroach that referenced this issue Oct 17, 2022
Fixes cockroachdb#86070

Release note (sql change): Star expressions, e.g., `SELECT * FROM ...`
are no longer allowed in statements in user-defined functions. They were
allowed in early betas of v22.2 from v22.2.0-beta.1 to v22.2.0-beta.4,
but have been disallowed because they do not behave correctly.
Issue cockroachdb#90080 tracks re-enabling star expressions in UDFs.
mgartner added a commit to mgartner/cockroach that referenced this issue Oct 17, 2022
This commit disallows star expressions in UDF bodies (see the release
note below). It also fixes an error message returned when trying to
create a view that references non-existent columns that incorrectly
mentioned star expressions.

Fixes cockroachdb#86070

Release note (sql change): Star expressions, e.g., `SELECT * FROM ...`
are no longer allowed in statements in user-defined functions. They were
allowed in early betas of v22.2 from v22.2.0-beta.1 to v22.2.0-beta.4,
but have been disallowed because they do not behave correctly.
Issue cockroachdb#90080 tracks re-enabling star expressions in UDFs.
mgartner added a commit to mgartner/cockroach that referenced this issue Oct 18, 2022
This commit disallows star expressions in UDF bodies (see the release
note below). It also fixes an error message returned when trying to
create a view that references non-existent columns that incorrectly
mentioned star expressions.

Fixes cockroachdb#86070

Release note (sql change): Star expressions, e.g., `SELECT * FROM ...`
are no longer allowed in statements in user-defined functions. They were
allowed in early betas of v22.2 from v22.2.0-beta.1 to v22.2.0-beta.4,
but have been disallowed because they do not behave correctly.
Issue cockroachdb#90080 tracks re-enabling star expressions in UDFs.
craig bot pushed a commit that referenced this issue Oct 18, 2022
90085: sql: disallow star expressions in UDF bodies r=mgartner a=mgartner

This commit disallows star expressions in UDF bodies (see the release
note below). It also fixes an error message returned when trying to
create a view that references non-existent columns that incorrectly
mentioned star expressions.

Fixes #86070

Release note (sql change): Star expressions, e.g., `SELECT * FROM ...`
are no longer allowed in statements in user-defined functions. They were
allowed in early betas of v22.2 from v22.2.0-beta.1 to v22.2.0-beta.4,
but have been disallowed because they do not behave correctly.
Issue #90080 tracks re-enabling star expressions in UDFs.


Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
blathers-crl bot pushed a commit that referenced this issue Oct 18, 2022
This commit disallows star expressions in UDF bodies (see the release
note below). It also fixes an error message returned when trying to
create a view that references non-existent columns that incorrectly
mentioned star expressions.

Fixes #86070

Release note (sql change): Star expressions, e.g., `SELECT * FROM ...`
are no longer allowed in statements in user-defined functions. They were
allowed in early betas of v22.2 from v22.2.0-beta.1 to v22.2.0-beta.4,
but have been disallowed because they do not behave correctly.
Issue #90080 tracks re-enabling star expressions in UDFs.
blathers-crl bot pushed a commit that referenced this issue Oct 18, 2022
This commit disallows star expressions in UDF bodies (see the release
note below). It also fixes an error message returned when trying to
create a view that references non-existent columns that incorrectly
mentioned star expressions.

Fixes #86070

Release note (sql change): Star expressions, e.g., `SELECT * FROM ...`
are no longer allowed in statements in user-defined functions. They were
allowed in early betas of v22.2 from v22.2.0-beta.1 to v22.2.0-beta.4,
but have been disallowed because they do not behave correctly.
Issue #90080 tracks re-enabling star expressions in UDFs.
@postamar postamar added the A-sql-routine UDFs and Stored Procedures label Nov 10, 2022
@exalate-issue-sync exalate-issue-sync bot removed the T-sql-queries SQL Queries Team label Nov 11, 2022
@mgartner
Copy link
Collaborator Author

mgartner commented Jan 10, 2023

In a previous issue #86070 (comment), @ajwerner said:

I think we can disable support for the * for this release and it would be okay. Postgres in views and in the eagerly resolved form of UDFs turns the * into column names at creation time. I think we should do that.

Here's an example of this behavior in Postgres (using the sql_body BEGIN ATOMIC syntax - we want this same behavior with the AS '<func_body>' syntax):

marcus=# CREATE TABLE t (a INT);
CREATE TABLE

marcus=# CREATE FUNCTION f() RETURNS INT LANGUAGE SQL BEGIN ATOMIC SELECT * FROM t; END;
CREATE FUNCTION

marcus=# INSERT INTO t VALUES (1);
INSERT 0 1

-- f() Returns the value of column a.
marcus=# SELECT f();
 f
---
 1
(1 row)

marcus=# ALTER TABLE t ADD COLUMN b INT DEFAULT 2;
ALTER TABLE

-- f() Still returns the value of column a.
marcus=# SELECT f();
 f
---
 1
(1 row)

marcus=# ALTER TABLE t RENAME COLUMN a TO c;
ALTER TABLE


-- f() Still returns the value of column c, which was
-- renamed from a.
marcus=# SELECT f();
 f
---
 1
(1 row)

marcus=# ALTER TABLE t RENAME COLUMN b TO a;
ALTER TABLE

-- f() Still returns the value of column c, even though
-- there is another column b.
marcus=# SELECT f();
 f
---
 1
(1 row)

rharding6373 added a commit to rharding6373/cockroach that referenced this issue Jan 24, 2023
This change allows `*` usage in UDF bodies. In order to facilitate
schema changes after a UDF is created but should not affect the UDF
output, we rewrite UDF ASTs in place to reference tables and columns by
ID instead of by name.

Informs: cockroachdb#90080

Epic: CRDB-19496
Release note (sql change): Allow `*` expressions in UDFs.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue Jan 24, 2023
This change allows `*` usage in UDF bodies. In order to facilitate
schema changes after a UDF is created but should not affect the UDF
output, we rewrite UDF ASTs in place to reference tables and columns by
ID instead of by name.

Informs: cockroachdb#90080

Epic: CRDB-19496
Release note (sql change): Allow `*` expressions in UDFs.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue Jan 25, 2023
This change allows `*` usage in UDF bodies. In order to facilitate
schema changes after a UDF is created but should not affect the UDF
output, we rewrite UDF ASTs in place to reference tables and columns by
ID instead of by name.

Informs: cockroachdb#90080

Epic: CRDB-19496
Release note (sql change): Allow `*` expressions in UDFs.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue Jan 26, 2023
This change allows `*` usage in UDF bodies. We rewrite UDF ASTs in place
to expand `*`s into the columns they reference.

Informs: cockroachdb#90080

Epic: CRDB-19496
Release note (sql change): Allow `*` expressions in UDFs.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue Jan 31, 2023
This change allows `*` usage in UDF bodies. We rewrite UDF ASTs in place
to expand `*`s into the columns they reference.

Informs: cockroachdb#90080

Epic: CRDB-19496
Release note (sql change): Allow `*` expressions in UDFs.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue Jan 31, 2023
This change allows `*` usage in UDF bodies. We rewrite UDF ASTs in place
to expand `*`s into the columns they reference.

Informs: cockroachdb#90080

Epic: CRDB-19496
Release note (sql change): Allow `*` expressions in UDFs.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue Feb 2, 2023
This change allows `*` usage in UDF bodies. We rewrite UDF ASTs in place
to expand `*`s into the columns they reference.

Informs: cockroachdb#90080

Epic: CRDB-19496
Release note (sql change): Allow `*` expressions in UDFs.
craig bot pushed a commit that referenced this issue Feb 3, 2023
95710: sql: support `*` in udf bodies r=rharding6373 a=rharding6373

This change allows `*` usage in UDF bodies. We rewrite UDF ASTs in place
to expand `*`s into the columns they reference.

Informs: #90080

Epic: CRDB-19496
Release note (sql change): Allow `*` expressions in UDFs.

96029: pkg/util/metric: option to use legacy hdrhistogram model, increase bucket fidelity r=tbg,aadityasondhi a=dhartunian

This patch reeintroduces the old HdrHistogram model to optionally be
enabled in favor of the new Prometheus model, gated behind
an environment variable called `COCKROACH_ENABLE_HDR_HISTOGRAMS`,
allowing users a means to "fall back" to the old model in the
event that the new model does not adequately serve their needs
(think of this as an "insurance policy" to protect against
this from happening again with no real mitigation - ideally,
this environment variable should never have to be used).

It also updates the pre-defined bucket boundaries used by the Prometheus
backed histograms with more buckets. This aims to improve precision,
especially for latency histograms, when calculating quantiles (the low precision
being the core cause of the issue at hand).

Note: some histograms were introduced *after* the new
Prometheus histograms were added to CockroachDB. In this
case, we use the `ForceUsePrometheus` option in the
`HistogramOptions` struct to ignore the value of the env
var, since there never was a time where these specific
histograms used the HdrHistogram model.

Release note (ops change): Histogram metrics can now optionally
use the legacy HdrHistogram model by setting the environment var
`COCKROACH_ENABLE_HDR_HISTOGRAMS=true` on CockroachDB nodes.
**Note that this is not recommended** unless users are having
difficulties with the newer Prometheus-backed histogram model.
Enabling can cause performance issues with timeseries databases
like Prometheus, as processing and storing the increased number
of buckets is taxing on both CPU and storage. Note that the
HdrHistogram model is slated for full deprecation in upcoming
releases.

Fixes: #95833

96269: sql/physicalplan: allow passing replica oracle to planning r=dt a=dt

This PR pulls out a subset of the changes in #93810, namely allowing a replica-choice oracle to be passed on a per-plan basis, but without changing which oracle is actually used in any existing planning, and only introducing the API for future users. 

This refactors span resolver and the physical planning methods to allow passing a replica-choice oracle per planning instead of using a fixed oracle (either bin-packing or closest depending on server type). The existing API that uses the default oracle is kept as is for all existing callers, while a new WithOracle variant is added for future callers who wish to pass their own oracle.

An new oracle that prefers followers over leaseholders is also added here, however it is not used at this time.

Release note: none.
Epic: none.

96349: logictest: skip another flaky test in ranges r=mgartner a=mgartner

This commit skips a flaky test that I missed in #96271.

Informs #96136

Epic: None

Release note: None

96433: storage: add string repr for FullReplicaID r=pavelkalinnikov a=tbg

Helpful for subsequent unit testing involving this type.

NB: this type is in the wrong place. Now's not the time
to move it.

Epic: none
Release note: None


96434: kvserver,kvstorage: move read/write methods for cluster versions r=pavelkalinnikov a=tbg

Same rationale as #95432 - they belong in `kvstorage` and I need
them there now as I'm working on a datadriven test.

Touches #93310.

Epic: CRDB-220
Release note: None


96445: backupccl: add 'aws-weekly' tag to restore/tpce/32tb test r=healthy-pod,renatolabs a=msbutler

Epic: none

Release note: None

Co-authored-by: rharding6373 <rharding6373@users.noreply.github.com>
Co-authored-by: David Hartunian <davidh@cockroachlabs.com>
Co-authored-by: Alex Barganier <abarganier@cockroachlabs.com>
Co-authored-by: Rui Hu <rui@cockroachlabs.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
Co-authored-by: Michael Butler <butler@cockroachlabs.com>
@rharding6373
Copy link
Collaborator

At this point we've added support for basic * expansion, but do not have support for the schema change edge cases (e.g., column and table renaming) mentioned in #90080 (comment). That should be supported when #83233 is resolved.

@mgartner mgartner closed this as completed Feb 7, 2023
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-routine UDFs and Stored Procedures C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) docs-known-limitation T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
Archived in project
Development

No branches or pull requests

4 participants