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

opt: unimplemented error for UDFs with mutations should reference an issue #99715

Closed
rytaft opened this issue Mar 27, 2023 · 0 comments · Fixed by #100965
Closed

opt: unimplemented error for UDFs with mutations should reference an issue #99715

rytaft opened this issue Mar 27, 2023 · 0 comments · Fixed by #100965
Assignees
Labels
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. E-quick-win Likely to be a quick win for someone experienced. O-qa T-sql-queries SQL Queries Team
Projects

Comments

@rytaft
Copy link
Collaborator

rytaft commented Mar 27, 2023

Describe the problem

UDFs don't currently support mutations, but the unimplemented error doesn't include a reference to an issue that tracks adding support.

To Reproduce

Run cockroach demo on master (or 23.1), and execute the following:

CREATE TABLE emp (name TEXT, salary INT);

CREATE FUNCTION clean_emp() RETURNS void AS '
    DELETE FROM emp
        WHERE salary < 0;
' LANGUAGE SQL;

This results in the error:

ERROR: unimplemented: DELETE usage inside a function definition
SQLSTATE: 0A000
HINT: You have attempted to use a feature that is not yet implemented.

Please check the public issue tracker to check whether this problem is
already tracked. If you cannot find it there, please report the error
with details by creating a new issue.

If you would rather not post publicly, please contact us directly
using the support form.

We appreciate your feedback.

Expected behavior

The unimplemented error should reference a public GitHub issue that tracks adding support for DELETEs in UDFs. If such an issue doesn't already exist, we should open an issue to track this (and other unsupported statements such as UPDATE, CREATE TABLE, etc) and reference it in the unimplemented error. It will also allow community members to post if they particularly want the feature.

Jira issue: CRDB-26034

@rytaft rytaft added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-qa labels Mar 27, 2023
@rytaft rytaft added this to Triage in SQL Queries via automation Mar 27, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Mar 27, 2023
@msirek msirek added A-sql-routine UDFs and Stored Procedures E-quick-win Likely to be a quick win for someone experienced. labels Mar 28, 2023
@msirek msirek moved this from Triage to 23.1 Release in SQL Queries Mar 28, 2023
@rharding6373 rharding6373 self-assigned this Apr 7, 2023
@rharding6373 rharding6373 moved this from 23.1 Release to Active in SQL Queries Apr 7, 2023
craig bot pushed a commit that referenced this issue Apr 10, 2023
100813: streamingccl: deflake TestRandomClientGeneration r=adityamaru a=msbutler

This patch fixes 4 bugs in TestRandomClientGeneration that were responsible for the persistent flakiness and lack of coverage in this test:
- the randomeStreamClient no longer instantiates keys with a table prefix that collides with the job info table prefix. This collision was the original cause of the flakes reported in #99343.
- getPartitionSpanToTableId() now generates a correct map from source partition key space to table Id. Previously, the key spans in the map didn't contain keys that mapped to anything logical in the cockroach key space.
- assertKVs() now checks for keys in the destination tenant keyspace.
- assertKVs() now actually asserts that kvs were found. Before, the assertion could pass if no keys were actually checked, which has been happening for months and allowed the bugs above to infest this test.

Fixes #99343

Release note: None

100952: cli: trash `TestNoLinkForbidden` r=rail a=rickystewart

This test does not work:
1. The test [has been broken](#74119) for years.
2. The test is not sensible in the Bazel world anyway, and under remote execution the test fails with an error like the following:

```
 build.go:59: go/build: go list github.com/cockroachdb/cockroach/pkg/cmd/cockroach: fork/exec GOROOT/bin/go: no such file or directory
```

The bug to replace this test with working functionality based on Bazel is #81526.

Epic: CRDB-17165
Release note: None
Closes #74119.

100965: sql: link issue to unimplemented mutations in udfs r=mgartner,rytaft a=rharding6373

Links an issue to the unimplemented errors for mutations in UDFs.

Epic: None
Informs: #87289
Fixes: #99715

Release note: None

Co-authored-by: Michael Butler <butler@cockroachlabs.com>
Co-authored-by: Ricky Stewart <rickybstewart@gmail.com>
Co-authored-by: rharding6373 <rharding6373@users.noreply.github.com>
@craig craig bot closed this as completed in 0d66c6c Apr 10, 2023
SQL Queries automation moved this from Active to Done Apr 10, 2023
rharding6373 added a commit to rharding6373/cockroach that referenced this issue Apr 10, 2023
Links an issue to the unimplemented errors for mutations in UDFs.

Epic: None
Informs: cockroachdb#87289
Fixes: cockroachdb#99715

Release note: None

Release justification: Changes an existing error message, so change is
not risky.
rharding6373 added a commit that referenced this issue Apr 10, 2023
Links an issue to the unimplemented errors for mutations in UDFs.

Epic: None
Informs: #87289
Fixes: #99715

Release note: None
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-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. O-qa T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants