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

Allow field name declaration in ROW literal #25261

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dain
Copy link
Member

@dain dain commented Mar 9, 2025

Description

Add support for row(a 1, b 2) instead of the much more complex cast(row(1, 2) as row(a integer, b integer)). The old syntax is particularly annoying because you have to repeat the types for the fields.

Release notes

(X) Release notes are required, with the following suggested text:

## General
* Allow field name declaration in ROW literal.  For example, `row(a 1, b 2)` is now legal. ({issue}`issuenumber`)

@dain dain requested a review from martint March 9, 2025 20:41
@cla-bot cla-bot bot added the cla-signed label Mar 9, 2025
@dain dain force-pushed the row-with-field-names-literal branch 3 times, most recently from 95e89ed to 6cbfff3 Compare March 10, 2025 05:09
@martint
Copy link
Member

martint commented Mar 10, 2025

I like this, but I would use the following syntax to make it similar to how the implicit row in the SELECT clause is constructed:

row(<expr> (AS? <identifier>)?, ...)

That way, the only syntactic difference with the SELECT clause, is that the fields are wrapped in row(...)

@martint
Copy link
Member

martint commented Mar 10, 2025

As another example, the syntax I suggested above makes these two equivalent:

SELECT 1 AS x, 2 AS y
VALUES row(1 AS x, 2 AS y)

@dain
Copy link
Member Author

dain commented Mar 11, 2025

As another example, the syntax I suggested above makes these two equivalent:

SELECT 1 AS x, 2 AS y
VALUES row(1 AS x, 2 AS y)

I'm working on the syntax change, but for VALUES the names of the relation aren't sourced from the rows. Today you can name row fields with a cast in values, but they don't effect the values column aliases:

values cast(row(1, 2) as row(foo bigint, bar bigint));
 _col0 | _col1 
-------+-------
     1 |     2 
(1 row)

I'm guessing that can be improved, but I have no idea how... work for follow up

@dain dain force-pushed the row-with-field-names-literal branch from 6cbfff3 to 129dee6 Compare March 11, 2025 01:39
Add support for `row(a 1, b 2)` instead of the much more complex
`cast(row(1, 2) as row(a integer, b integer))`.
@dain dain force-pushed the row-with-field-names-literal branch from 129dee6 to 85c7a67 Compare March 11, 2025 03:01
import java.util.stream.Collectors;

import static java.util.Objects.requireNonNull;

@JsonSerialize
public record Row(List<Expression> items)
public record Row(List<Field> fields)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't be necessary to add the field names in the IR. As far as I recall, there's nothing in the planner/optimizer that can or needs to use them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants