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

Fix table materialization for Delta models #420

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

Jeremynadal33
Copy link
Contributor

@Jeremynadal33 Jeremynadal33 commented Aug 6, 2024

resolves #419 with the first (maybe not the best) idea

Description

I modified the function get_relation to pass the argument is_delta to the relation's creation.
Then, I added a "or replace" in the glue__create_table_as to be able to rebuild multiple times the same model.

I am pretty sure we could update the is_iceberg and is_hudi arguments in the same way hence render the iceberg_table_replace useless but I did not have time to test it out

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-glue next" section.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

FYI :

  • I am not sure wether this change needs testing, if it does, I would be interested in understanding how to!

Tests that were run locally

Simple model with minimal table strategy (test_table):

{{
    config(
        materialized='table',
        file_format='delta',
    )
}}

with incoming_data as (
    select 1 as id
)

select * from incoming_data

No problem at first run but when running a second time the following command :

dbt run -s test_table

Got the following error :

15:25:56  Glue adapter: Glue returned `error` for statement None for code SqlWrapper2.execute('''/* {"app": "dbt", "dbt_version": "1.8.4", "profile_name": "data_platform_preparation", "target_name": "local", "node_id": "model.data_platform_preparation.test_table"} */

    create table xxx.test_table
    using delta
        LOCATION 's3://xxx/xxx/test_table/'
        as
with incoming_data as (
    select 1 as id
)

select * from incoming_data
  ''', use_arrow=False, location='s3://xxx'), AnalysisException: Cannot create table ('`lgdev_jnadal`.`test_table`'). The associated location ('s3://xxx/xxx/test_table') is not empty but it's not a Delta table

Now I can rebuild the model without problem, change its schema also works.

@Jeremynadal33 Jeremynadal33 changed the title added delta_table_replace materialization Added delta_table_replace materialization Aug 6, 2024
@Jeremynadal33 Jeremynadal33 force-pushed the feature/allow-delta-table branch from 85505d0 to 3e43926 Compare August 6, 2024 15:28
@Jeremynadal33 Jeremynadal33 changed the title Added delta_table_replace materialization Fix table materialization for Delta models Aug 8, 2024
@Jeremynadal33 Jeremynadal33 force-pushed the feature/allow-delta-table branch from 9f09d30 to a855e39 Compare August 8, 2024 09:18
@moomindani moomindani added the enable-functional-tests This label enable functional tests label Aug 26, 2024
Copy link
Collaborator

@moomindani moomindani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me.

@moomindani moomindani merged commit aeea602 into aws-samples:main Aug 26, 2024
19 checks passed
@moomindani
Copy link
Collaborator

Thanks for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor enable-functional-tests This label enable functional tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to rebuild a Delta model materialized as table
2 participants