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: RETURNS TABLE not supported for UDFs #100226

Closed
rytaft opened this issue Mar 30, 2023 · 2 comments · Fixed by #137251
Closed

sql: RETURNS TABLE not supported for UDFs #100226

rytaft opened this issue Mar 30, 2023 · 2 comments · Fixed by #137251
Labels
A-sql-udf C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-qa T-sql-queries SQL Queries Team

Comments

@rytaft
Copy link
Collaborator

rytaft commented Mar 30, 2023

Describe the problem

Trying to create a UDF with RETURNS TABLE currently results in a syntax error.

To Reproduce

Run the following in cockroach demo on master / 23.1:

CREATE FUNCTION sum_n_product_with_tab (x int)
RETURNS TABLE(sum int, product int) AS $$
    SELECT $1 + tab.y, $1 * tab.y FROM tab;
$$ LANGUAGE SQL;

On CRDB it results in this error:

invalid syntax: statement ignored: at or near "table": syntax error
SQLSTATE: 42601
DETAIL: source SQL:
CREATE FUNCTION sum_n_product_with_tab (x int)
RETURNS TABLE(sum int, product int) AS $$
        ^
HINT: try \h CREATE FUNCTION

Expected behavior

This works on Postgres, so we should support it. But in the meantime, at least we should return an unimplemented error with an appropriate issue number.

Jira issue: CRDB-26307

gz#20015

@rytaft rytaft added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-qa A-sql-routine UDFs and Stored Procedures labels Mar 30, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Mar 30, 2023
@rytaft
Copy link
Collaborator Author

rytaft commented Mar 30, 2023

@mgartner I put this in 23.1 since I'm not sure if you care about telemetry for this feature? If not, feel free to move this to backlog.

mgartner added a commit to mgartner/cockroach that referenced this issue Apr 3, 2023
@mgartner mgartner assigned mgartner and unassigned mgartner Apr 3, 2023
craig bot pushed a commit that referenced this issue Apr 4, 2023
100539: sql: add telemetry for UDFs with RETURNS TABLE r=mgartner a=mgartner

Informs #100226

Release note: None

100554: kv: initialize consistencyLimiter during Store construction, before Start r=aliher1911 a=nvanbenschoten

Fixes #96231.

This commit attempts to fix #96231. It moves the initialization of `Store.consistencyLimiter` up from the bottom of `Store.Start` into `NewStore`. It is unsafe to initialize this variable after the call to `Store.processRaft`, which starts Raft processing. Beyond that point, incoming Raft traffic is permitted and calls to `computeChecksumPostApply` are possible.

The two stacktraces we have in that issue conclusively point to the `Store.consistencyLimiter` being nil during a call to `(*Replica).computeChecksumPostApply`. This startup-time race is the only possible explanation I could come up with.

Release note (bug fix): Fixed a rare startup race that could cause an `invalid memory address or nil pointer dereference` error.

100592: storage: remove unnecessary version override in unit test r=nicktrav a=jbowens

Remove an unnecessary use of cluster.MakeTestingClusterSettingsWithVersions in favor of cluster.MakeTestingClusterSettings.

Epic: None
Informs: #100552
Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
blathers-crl bot pushed a commit that referenced this issue Apr 4, 2023
@mgartner mgartner moved this to New Backlog in SQL Queries Jul 24, 2023
@giangpham712
Copy link

@fqazi This affects efcore.pg tests

@exalate-issue-sync exalate-issue-sync bot added A-sql-udf and removed A-sql-routine UDFs and Stored Procedures labels Sep 18, 2024
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Dec 11, 2024
This commit implements `RETURNS TABLE` for UDFs. `RETURNS TABLE` is
syntactic sugar for `RETURNS SETOF` with:
- RECORD if there are multiple TABLE parameters, or
- the type of the single TABLE parameter.
The TABLE parameters are added to the list of routine parameters.

Fixes cockroachdb#100226

Release note (sql change): Added support for `RETURNS TABLE` syntax when
creating a UDF.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Dec 12, 2024
This commit implements `RETURNS TABLE` for UDFs. `RETURNS TABLE` is
syntactic sugar for `RETURNS SETOF` with:
- RECORD if there are multiple TABLE parameters, or
- the type of the single TABLE parameter.
The TABLE parameters are added to the list of routine parameters.

Fixes cockroachdb#100226

Release note (sql change): Added support for `RETURNS TABLE` syntax when
creating a UDF.
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Dec 12, 2024
This commit implements `RETURNS TABLE` for UDFs. `RETURNS TABLE` is
syntactic sugar for `RETURNS SETOF` with:
- RECORD if there are multiple TABLE parameters, or
- the type of the single TABLE parameter.
The TABLE parameters are added to the list of routine parameters.

Fixes cockroachdb#100226

Release note (sql change): Added support for `RETURNS TABLE` syntax when
creating a UDF.
craig bot pushed a commit that referenced this issue Dec 12, 2024
137251: sql: implement RETURNS TABLE syntax r=DrewKimball a=DrewKimball

#### sql: do not limit set-returning UDF when it has OUT parameters

This commit fixes a bug that caused the `SetOf` option for the UDF
`ReturnType` to be overwritten if the UDF had OUT parameters. The bug
caused a `LIMIT 1` to be imposed on the UDF's final body statement, so
that the UDF returned only a single row.

Fixes #128403

Release note (bug fix): Fixed a bug existing since v24.1 that would
cause a set-returning UDF with OUT parameters to return a single row.

#### sql: implement RETURNS TABLE syntax

This commit implements `RETURNS TABLE` for UDFs. `RETURNS TABLE` is
syntactic sugar for `RETURNS SETOF` with:
- RECORD if there are multiple TABLE parameters, or
- the type of the single TABLE parameter.
The TABLE parameters are added to the list of routine parameters.

Fixes #100226

Release note (sql change): Added support for `RETURNS TABLE` syntax when
creating a UDF.

Co-authored-by: Drew Kimball <[email protected]>
craig bot pushed a commit that referenced this issue Dec 13, 2024
137251: sql: implement RETURNS TABLE syntax r=DrewKimball a=DrewKimball

#### sql: do not limit set-returning UDF when it has OUT parameters

This commit fixes a bug that caused the `SetOf` option for the UDF
`ReturnType` to be overwritten if the UDF had OUT parameters. The bug
caused a `LIMIT 1` to be imposed on the UDF's final body statement, so
that the UDF returned only a single row.

Fixes #128403

Release note (bug fix): Fixed a bug existing since v24.1 that would
cause a set-returning UDF with OUT parameters to return a single row.

#### sql: implement RETURNS TABLE syntax

This commit implements `RETURNS TABLE` for UDFs. `RETURNS TABLE` is
syntactic sugar for `RETURNS SETOF` with:
- RECORD if there are multiple TABLE parameters, or
- the type of the single TABLE parameter.
The TABLE parameters are added to the list of routine parameters.

Fixes #100226

Release note (sql change): Added support for `RETURNS TABLE` syntax when
creating a UDF.

Co-authored-by: Drew Kimball <[email protected]>
@craig craig bot closed this as completed in 074371d Dec 13, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in SQL Queries Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-udf C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-qa T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants