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: support COLLATE expressions on arrays #133751

Merged
merged 1 commit into from
Nov 27, 2024
Merged

Conversation

michae2
Copy link
Collaborator

@michae2 michae2 commented Oct 29, 2024

In PG, COLLATE is a scalar operator that modifies its operand to produce a collated type. This commit adds support for COLLATE on arrays of strings, which marks them as arrays of collated strings.

Epic: None

Release note (sql change): Add support for COLLATE expressions on arrays of strings, to better match PostgreSQL.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@michae2 michae2 force-pushed the collated branch 2 times, most recently from 4ec8e74 to ba2b61f Compare November 20, 2024 00:39
@michae2 michae2 requested review from mgartner, mw5h and a team November 20, 2024 00:40
@michae2 michae2 marked this pull request as ready for review November 20, 2024 00:40
@michae2 michae2 requested a review from a team as a code owner November 20, 2024 00:40
@michae2
Copy link
Collaborator Author

michae2 commented Nov 20, 2024

This came up during an escalation a couple weeks ago. It probably needs a few more tests, but should be RFAL.

Copy link
Collaborator

@mgartner mgartner left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @mw5h)


pkg/sql/logictest/testdata/logic_test/collatedstring line 616 at r1 (raw file):

subtest collate_array

# Test collation of string arrays.

Can you add some nested array test cases, e..g, SELECT '{{a}}'::TEXT[][] COLLATE "...";.

Copy link
Collaborator Author

@michae2 michae2 left a comment

Choose a reason for hiding this comment

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

Thanks!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @mgartner and @mw5h)


pkg/sql/logictest/testdata/logic_test/collatedstring line 616 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

Can you add some nested array test cases, e..g, SELECT '{{a}}'::TEXT[][] COLLATE "...";.

This is a little tricky because of #32552. I did add a few, but the results don't exactly match PG due to #36815.

Copy link
Collaborator

@mgartner mgartner left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mw5h)

In PG, COLLATE is a scalar operator that modifies its operand to produce
a collated type. This commit adds support for COLLATE on arrays of
strings, which marks them as arrays of collated strings.

Epic: None

Release note (sql change): Add support for COLLATE expressions on arrays
of strings, to better match PostgreSQL.
@michae2
Copy link
Collaborator Author

michae2 commented Nov 26, 2024

TFTR!

bors r=mgartner

craig bot pushed a commit that referenced this pull request Nov 26, 2024
133751: sql: support COLLATE expressions on arrays r=mgartner a=michae2

In PG, COLLATE is a scalar operator that modifies its operand to produce a collated type. This commit adds support for COLLATE on arrays of strings, which marks them as arrays of collated strings.

Epic: None

Release note (sql change): Add support for COLLATE expressions on arrays of strings, to better match PostgreSQL.

136173: workload: handle timestamp native type in CSV writer and a benchmark r=yuzefovich a=yuzefovich

In ef3946c we forgot to update two spots to handle the native timestamp type in the CSV writer and a benchmark.

Fixes: #135975.
Fixes: #135979.
Fixes: #135984.

Release note: None

Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
@michae2
Copy link
Collaborator Author

michae2 commented Nov 26, 2024

looks like this was cancelled, trying again

bors r=mgartner

@craig craig bot merged commit 5ecd537 into cockroachdb:master Nov 27, 2024
22 of 23 checks passed
@michae2 michae2 deleted the collated branch November 27, 2024 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants