-
Notifications
You must be signed in to change notification settings - Fork 178
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
Fix the str_flatten
function in Redshift
#805
Conversation
order <- win_current_order() | ||
if(length(order) > 0){ | ||
sql <- build_sql(sql_expr(LISTAGG(!!x, !!collapse)), | ||
" WITHIN GROUP (ORDER BY ", order, ")") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there other functions in redshift that use this syntax? I'm surprised that (say) windowed mean()
doesn't need the same syntax? Or is LISTAGG()
the same sort of function as PERCENTILE_DISC()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From searching the documentation only the LISTAGG()
, PERCENTILE_DISC()
, PERCENTILE_CONT()
, and ST_COLLECT()
functions in redshift require the WITHIN GROUP
clause for the ordering. The remaining windowed functions use the more general OVER (PARTITION BY ... ORDER BY ...)
clause.
Unfortunately, I cannot explain why LISTAGG()
in particular uses the WITHIN GROUP
syntax. But this syntax seems to be shared by other database types (e.g. Oracle).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the investigation! Unfortunately there seem to be few satisfying explanation for why SQL things are the way they are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a bullet to the top of NEWS.md
? It should briefly describe the change and end with (@yourname, #issuenumber)
.
Just added the bullet to the NEWS.md. Thanks @hadley. |
Thanks! |
Fixes #804
This pull request adds a Redshift specific translation to
str_flatten
. Currently the default Postgres translation (string_agg
) is used, but it is not supported in Redshift. The equivalent function in Redshift isLISTAGG
but it has a slightly different syntax (requiresWITHIN GROUP (ORDER BY ...)
syntax for ordering).Please check below the reprex with the expected output:
Created on 2022-03-25 by the reprex package (v2.0.0)