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: implement string_agg() #26737

Closed
BramGruneir opened this issue Jun 14, 2018 · 3 comments
Closed

sql: implement string_agg() #26737

BramGruneir opened this issue Jun 14, 2018 · 3 comments
Assignees
@BramGruneir
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-hibernate Issues that pertain to Hibernate integration. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Milestone
2.1

Comments

@BramGruneir
Copy link
Member

BramGruneir commented Jun 14, 2018 •
edited
Loading

Every hibernate test tries to fetch the connection metadata and uses string_agg() as part of it.

While it is not critical to pass the tests, there is no reason why we shouldn't just add the function and there's a todo to so in the code here:

cockroach/pkg/sql/sem/builtins/aggregate_builtins.go

Line 137 in e35c4c8

// TODO(knz): When CockroachDB supports STRING_AGG, CONCAT_AGG(X) 

 WARN JdbcEnvironmentInitiator:121 - HHH000341: Could not obtain connection metadata : ERROR: unknown function: string_agg()
@BramGruneir BramGruneir added A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-hibernate Issues that pertain to Hibernate integration. labels Jun 14, 2018
@BramGruneir BramGruneir mentioned this issue Jun 14, 2018
Closed
14 tasks
@knz knz mentioned this issue Jun 19, 2018
Closed
@knz
Copy link
Contributor

knz commented Jun 19, 2018

This is blocked on #10495.

@knz knz added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jun 19, 2018
@olle
Copy link

olle commented Jul 6, 2018

Hi, I ran into this issue with Spring Boot 2.x, managing schemas using Liquibase - even without Hibernate or JPA enabled.

Sorry for the lack of info. Just though it could be of interest. Thanks!

@knz knz added this to the 2.1 milestone Jul 23, 2018
BramGruneir added a commit to BramGruneir/cockroach that referenced this issue Aug 8, 2018
@BramGruneir
sql: add the string_agg aggregation function
83f08d5 
This function is similar to concat_agg but it takes a delimiter as a secondary
argument. Previously, we were not able to handle aggregations with more than
one argument before. To allow for this, without getting into the messy world of
multi-column aggregators, all arguments after the first one in an aggregator
must be constant expressions.

This in turn required updating the aggregator functions to now also take
argument datums as an new argument.

For distsql, the arguments are stored as expressions that have already been
checked to ensure that they are constants.

It looks like concat_agg (and now string_agg) are not run in distsql yet, so
this will be added next.

This work is primarily motivated by the need for greater ORM compatibility.

Closes cockroachdb#10495, cockroachdb#26737

Release note (sql change): Added the new aggregation function string_agg that
concats a collection of strings into a single string and seperates them with the
passed in delimiter.
@BramGruneir BramGruneir mentioned this issue Aug 8, 2018
Merged
BramGruneir added a commit to BramGruneir/cockroach that referenced this issue Aug 9, 2018
@BramGruneir
sql: add the string_agg aggregation function
29029d2 
This function is similar to concat_agg but it takes a delimiter as a secondary
argument. Previously, we were not able to handle aggregations with more than
one argument before. To allow for this, without getting into the messy world of
multi-column aggregators, all arguments after the first one in an aggregator
must be constant expressions.

This in turn required updating the aggregator functions to now also take
argument datums as an new argument.

For distsql, the arguments are stored as expressions that have already been
checked to ensure that they are constants.

It looks like concat_agg (and now string_agg) are not run in distsql yet, so
this will be added next.

This work is primarily motivated by the need for greater ORM compatibility.

Closes cockroachdb#10495, cockroachdb#26737

Release note (sql change): Added the new aggregation function string_agg that
concats a collection of strings into a single string and seperates them with the
passed in delimiter.
BramGruneir added a commit to BramGruneir/cockroach that referenced this issue Aug 13, 2018
@BramGruneir
sql: add the string_agg aggregation function
599dc19 
This function is similar to concat_agg but it takes a delimiter as a secondary
argument. Previously, we were not able to handle aggregations with more than
one argument before. To allow for this, without getting into the messy world of
multi-column aggregators, all arguments after the first one in an aggregator
must be constant expressions.

This in turn required updating the aggregator functions to now also take
argument datums as an new argument.

For distsql, the arguments are stored as expressions that have already been
checked to ensure that they are constants.

However, when used as a window function, the limitation of not having more than
one non-const expression is not required.

It looks like concat_agg (and now string_agg) are not run in distsql yet, so
this will be added next.

This work is primarily motivated by the need for greater ORM compatibility.

Closes cockroachdb#10495, cockroachdb#26737

Release note (sql change): Added the new aggregation function string_agg that
concats a collection of strings into a single string and seperates them with the
passed in delimiter.
BramGruneir added a commit to BramGruneir/cockroach that referenced this issue Aug 14, 2018
@BramGruneir
sql: add the string_agg aggregation function
7198227 
This function is similar to concat_agg but it takes a delimiter as a secondary
argument. Previously, we were not able to handle aggregations with more than
one argument before. To allow for this, without getting into the messy world of
multi-column aggregators, all arguments after the first one in an aggregator
must be constant expressions.

This in turn required updating the aggregator functions to now also take
argument datums as an new argument.

For distsql, the arguments are stored as expressions that have already been
checked to ensure that they are constants.

However, when used as a window function, the limitation of not having more than
one non-const expression is not required.

It looks like concat_agg (and now string_agg) are not run in distsql yet, so
this will be added next.

This work is primarily motivated by the need for greater ORM compatibility.

Closes cockroachdb#10495, cockroachdb#26737

Release note (sql change): Added the new aggregation function string_agg that
concats a collection of strings into a single string and seperates them with the
passed in delimiter.
BramGruneir added a commit to BramGruneir/cockroach that referenced this issue Aug 14, 2018
@BramGruneir
sql: add the string_agg aggregation function
1e8b654 
This function is similar to concat_agg but it takes a delimiter as a secondary
argument. Previously, we were not able to handle aggregations with more than
one argument before. To allow for this, without getting into the messy world of
multi-column aggregators, all arguments after the first one in an aggregator
must be constant expressions.

This in turn required updating the aggregator functions to now also take
argument datums as an new argument.

For distsql, the arguments are stored as expressions that have already been
checked to ensure that they are constants.

However, when used as a window function, the limitation of not having more than
one non-const expression is not required.

It looks like concat_agg (and now string_agg) are not run in distsql yet, so
this will be added next.

This work is primarily motivated by the need for greater ORM compatibility.

Closes cockroachdb#10495, cockroachdb#26737

Release note (sql change): Added the new aggregation function string_agg that
concats a collection of strings into a single string and seperates them with the
passed in delimiter.
BramGruneir added a commit to BramGruneir/cockroach that referenced this issue Aug 14, 2018
@BramGruneir
sql: add the string_agg aggregation function
0471171 
This function is similar to concat_agg but it takes a delimiter as a secondary
argument. Previously, we were not able to handle aggregations with more than
one argument before. To allow for this, without getting into the messy world of
multi-column aggregators, all arguments after the first one in an aggregator
must be constant expressions.

This in turn required updating the aggregator functions to now also take
argument datums as an new argument.

For distsql, the arguments are stored as expressions that have already been
checked to ensure that they are constants.

However, when used as a window function, the limitation of not having more than
one non-const expression is not required.

It looks like concat_agg (and now string_agg) are not run in distsql yet, so
this will be added next.

This work is primarily motivated by the need for greater ORM compatibility.

Closes cockroachdb#10495, cockroachdb#26737

Release note (sql change): Added the new aggregation function string_agg that
concats a collection of strings into a single string and seperates them with the
passed in delimiter.
craig bot pushed a commit that referenced this issue Aug 14, 2018
@BramGruneir @bdarnell
Merge #28392 #28470
334d178 
28392: sql: add the string_agg aggregation function r=BramGruneir a=BramGruneir

This function is similar to concat_agg but it takes a delimiter as a secondary
argument. Previously, we were not able to handle aggregations with more than
one argument before. To allow for this, without getting into the messy world of
multi-column aggregators, all arguments after the first one in an aggregator
must be constant expressions.

This in turn required updating the aggregator functions to now also take
argument datums as an new argument.

For distsql, the arguments are stored as expressions that have already been
checked to ensure that they are constants.

It looks like concat_agg (and now string_agg) are not run in distsql yet, so
this will be added next.

This work is primarily motivated by the need for greater ORM compatibility.

Closes #10495, #26737

Release note (sql change): Added the new aggregation function string_agg that
concats a collection of strings into a single string and seperates them with the
passed in delimiter.

28470: hlc: Prevent false positives in forward clock jump monitoring r=tschottdorf a=bdarnell

A recent change to the ordering of various startup events has led to a
gap between calls to hlc.Clock.Now, which looks like a forward clock
jump to first iteration of the monitoring goroutine.

Fixes #28367

Release note: None

Co-authored-by: Bram Gruneir <bram@cockroachlabs.com>
Co-authored-by: Ben Darnell <ben@bendarnell.com>
@BramGruneir
Copy link
Member Author

This is done!

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

@BramGruneir BramGruneir

Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-hibernate Issues that pertain to Hibernate integration. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
2.1
Development

No branches or pull requests
3 participants
@olle @knz @BramGruneir