dbt-labs / dbt-core

dbt enables data analysts and engineers to transform their data using the same practices that software engineers use to build applications.
https://getdbt.com
Apache License 2.0
9.25k stars 1.53k forks source link

Support for redshift 821 #10366

Open VersusFacit opened 3 days ago

VersusFacit commented 3 days ago

Resolves #10365

Problem

Solution

Sort and do some like validation for nulls.

Checklist

github-actions[bot] commented 3 days ago

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

codecov[bot] commented 3 days ago

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.76%. Comparing base (d936a63) to head (0671f85).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #10366 +/- ## ========================================== - Coverage 88.78% 88.76% -0.02% ========================================== Files 180 180 Lines 22487 22492 +5 ========================================== Hits 19965 19965 - Misses 2522 2527 +5 ``` | [Flag](https://app.codecov.io/gh/dbt-labs/dbt-core/pull/10366/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dbt-labs) | Coverage Δ | | |---|---|---| | [integration](https://app.codecov.io/gh/dbt-labs/dbt-core/pull/10366/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dbt-labs) | `86.06% <80.00%> (-0.06%)` | :arrow_down: | | [unit](https://app.codecov.io/gh/dbt-labs/dbt-core/pull/10366/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dbt-labs) | `62.19% <80.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dbt-labs#carryforward-flags-in-the-pull-request-comment) to find out more. | [Components](https://app.codecov.io/gh/dbt-labs/dbt-core/pull/10366/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dbt-labs) | Coverage Δ | | |---|---|---| | [Unit Tests](https://app.codecov.io/gh/dbt-labs/dbt-core/pull/10366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dbt-labs) | `62.19% <80.00%> (+<0.01%)` | :arrow_up: | | [Integration Tests](https://app.codecov.io/gh/dbt-labs/dbt-core/pull/10366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dbt-labs) | `86.06% <80.00%> (-0.06%)` | :arrow_down: |
colin-rogers-dbt commented 1 day ago

@MichelleArk curious if this issue might be a code smell for us to eventually move some of this functionality into the base adapter? That would enable different adapters to handle these kinds of edge cases on their own

VersusFacit commented 1 day ago

Colin, if we could move even just the final sql rendering into the base adapter, I could keep all this logic out of core and provide more logically interfaces for testing. So that's an enthusiastic +1 from me.

I did explore some additional options for accessing the rows before they are rendered as SQL in the base adapter but no dice.