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

[CT-1223] [Feature] consolidate listagg macros from packages and dbt core #5898

Closed
3 tasks done
dave-connors-3 opened this issue Sep 21, 2022 · 3 comments
Closed
3 tasks done
Labels
awaiting_response enhancement New feature or request stale Issues that have gone stale Team:Adapters Issues designated for the adapter area of the code

Comments

@dave-connors-3
Copy link
Contributor

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt functionality, rather than a Big Idea better suited to a discussion

Describe the feature

right now in dbt_project_evaluator, we are overriding the dbt-core version of listagg to allow for sorting the values in the array. We do not allow for a limit clause.

The core version on the other hand does the opposite! It allows for limiting, but not for sorting!

We really only rely on the listagg macro sorting to ensure our model outputs match seeds. Without having looked super closely, there's probably some ways for us to use the core version and still have passing tests, but wanted to at least open a thread here on what the right way to reconcile these macros would be!

Describe alternatives you've considered

the alternative would be changing our project to use the core version, which, frankly is completely reasonable

Who will this benefit?

package maintainers (specifically, @graciegoheen @b-per et al)

Are you interested in contributing this feature?

sure

Anything else?

nah

@dave-connors-3 dave-connors-3 added enhancement New feature or request triage labels Sep 21, 2022
@github-actions github-actions bot changed the title [Feature] consolidate listagg macros from packages and dbt core [CT-1223] [Feature] consolidate listagg macros from packages and dbt core Sep 21, 2022
@jtcohen6
Copy link
Contributor

@dave-connors-3 Thanks for opening!

Is this specific to Spark/Databricks?

The core version of this macro does support order_by_clause, and that works on most adapters / data platforms, but it's not supported in SparkSQL. You're right that SparkSQL does support array_sort — which can sort alphabetically, or with a custom case when expression (!), but only based on the values already in the array, not based on the value of another column (the real benefit of order_by_clause).

Relevant thread from June, wherein Doug and I were briefly spelunking down this rabbit hole: #5298 (comment)

@jtcohen6 jtcohen6 added Team:Adapters Issues designated for the adapter area of the code awaiting_response and removed triage labels Sep 21, 2022
@github-actions
Copy link
Contributor

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Dec 21, 2022
@github-actions
Copy link
Contributor

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting_response enhancement New feature or request stale Issues that have gone stale Team:Adapters Issues designated for the adapter area of the code
Projects
None yet
Development

No branches or pull requests

2 participants