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.

Sign up for GitHub

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

Jump to bottom

sql: max/min aggregates don't work with collated strings #45846

Closed
andy-kimball opened this issue Mar 8, 2020 · 3 comments · Fixed by #46647 or #49530
Closed

sql: max/min aggregates don't work with collated strings #45846

andy-kimball opened this issue Mar 8, 2020 · 3 comments · Fixed by #46647 or #49530
Assignees
@jordanlewis
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@andy-kimball
Copy link
Contributor

Repro:

root@:26257/defaultdb> SELECT max(x) FROM (VALUES ('foo' COLLATE en), ('bar' COLLATE en)) t(x);
ERROR: unknown signature: max(collatedstring{en})

EXPECTED: This should work; it does in PG.
@andy-kimball
Copy link
Contributor Author

Maybe good first issue?

@awoods187 awoods187 added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) good first issue labels Mar 9, 2020
rohany added a commit to rohany/cockroach that referenced this issue Mar 26, 2020
@rohany
sql: add support for max/min aggregates on collated strings
0d282d1 
Fixes cockroachdb#45846.

Release justification: low risk update to existing functionality
Release note (sql change): The max/min aggregates now work on collated
strings.
@rohany rohany mentioned this issue Mar 26, 2020
Merged
craig bot pushed a commit that referenced this issue Mar 26, 2020
@rohany
Merge #46647
fb7ae83 
46647: sql: add support for max/min aggregates on collated strings r=yuzefovich a=rohany

Fixes #45846.

Release justification: low risk update to existing functionality
Release note (sql change): The max/min aggregates now work on collated
strings.

Co-authored-by: Rohan Yadav <[email protected]>
@craig craig bot closed this as completed in 9b84571 Mar 26, 2020
@rohany rohany reopened this Mar 27, 2020
@jordanlewis
Copy link
Member

Why re-opened @rohany ?

@jordanlewis
Copy link
Member

Ah. #46693

@rohany rohany mentioned this issue May 25, 2020
Merged
rohany added a commit to rohany/cockroach that referenced this issue May 26, 2020
@rohany
sql: support max/min aggregates on collated strings
9afee91 
Fixes cockroachdb#45846.
Fixes cockroachdb#46685.

This PR allows for performing the max/min aggregations on collated
strings. Currently, our aggregates would not support container types
like `AnyCollatedString`. This PR extends the aggregates infrastructure
in DistSQL that caused the limitation to allow for container types,
which unblocks cockroachdb#48370.

Release note (sql change): Support the max/min aggregations on collated
strings.
craig bot pushed a commit that referenced this issue May 27, 2020
@rohany @dt @yuzefovich
Merge #49307 #49530 #49567 #49574
126df91 
49307: sql: add support for virtual schema qualified types r=rohany a=rohany

Fixes #16395.

Release note (sql change): Allows for referencing static data types
under the `pg_catalog` qualification like `pg_catalog.int`.

49530: sql: support max/min aggregates on collated strings r=rohany a=rohany

Fixes #45846.
Fixes #46685.

This PR allows for performing the max/min aggregations on collated
strings. Currently, our aggregates would not support container types
like `AnyCollatedString`. This PR extends the aggregates infrastructure
in DistSQL that caused the limitation to allow for container types,
which unblocks #48370.

Release note (sql change): Support the max/min aggregations on collated
strings.

49567: cloud: bump version to 20.1.1 r=dt a=dt

Release note: none.

49574: roachtest: get bigger machines for alterpk-tpcc-250 r=yuzefovich a=yuzefovich

The test has been failing for a while now because we're operating very
close to memory limit. Usually, 3.3.2.6 expensive check query ends up
crashing the node, sometimes it hits SQL memory limit. I've been staring
at it for a while, but there is a discrepancy between the memory usage
reported in runtime stats in logs and the usage that shows up in heap
profiles (for example, I could see 8GB in the former and 3GB in the
latter), and I couldn't figure out where that gaps goes. I don't think
spending any more time on this issue is worthwhile, so this commit bumps
the machine type from cpu-16 to cpu-32 which doubles available RAM from
14.4 to 28.8.

Fixes: #48428.

Release note: None

Co-authored-by: Rohan Yadav <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
@craig craig bot closed this as completed in 974be22 May 27, 2020
jbowens pushed a commit to jbowens/cockroach that referenced this issue Jun 1, 2020
@rohany @jbowens
sql: support max/min aggregates on collated strings
c1c7b89 
Fixes cockroachdb#45846.
Fixes cockroachdb#46685.

This PR allows for performing the max/min aggregations on collated
strings. Currently, our aggregates would not support container types
like `AnyCollatedString`. This PR extends the aggregates infrastructure
in DistSQL that caused the limitation to allow for container types,
which unblocks cockroachdb#48370.

Release note (sql change): Support the max/min aggregations on collated
strings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jordanlewis jordanlewis

Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
4 participants
@jordanlewis @rohany @awoods187 @andy-kimball