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

feat: use glue api to delete table instead of athena drop #249

Merged
merged 2 commits into from
May 4, 2023

Conversation

chrischin478
Copy link
Contributor

@chrischin478 chrischin478 commented Apr 23, 2023

Description

Feature #44. Use Glue API instead of Athena to drop tables and views.

Models used to test

Table
{{ config(materialized='table') }}

WITH source_data AS (
    SELECT 1 AS id
    UNION ALL
    SELECT NULL AS id
)
SELECT *
FROM source_data
Iceberg Table
{{ config(
    materialized='table',
    table_type='iceberg',
    format='parquet'
) }}

SELECT
  'A' AS user_id,
  'pi' AS name,
  'active' AS status,
  17.89 AS cost,
  1 AS quantity,
  100000000 AS quantity_big,
  current_date AS my_date
Hive HA Table
{{ config(
    materialized='table_hive_ha',
    format='parquet',
    partition_by=['status'],
    s3_data_naming='table_unique'
) }}

SELECT
  'a' as user_id,
  'pi' as user_name,
  'active' as status
UNION ALL
SELECT
  'b' as user_id,
  'sh' as user_name,
  'disabled' as status
Incremental Table
{{ 
    config(
        materialized='incremental',
        unique_key='id'
    ) 
}}

SELECT
    1 AS id,
    'a' AS key_value
UNION ALL
SELECT
    2 AS id,
    'b' AS key_value

Checklist

  • You followed contributing section
  • You added unit testing when necessary
  • You added functional testing when necessary

@chrischin478
Copy link
Contributor Author

I'm not sure if any test cases need to be added. I haven't yet tested an incremental table but am planning to tomorrow.

@nicor88
Copy link
Member

nicor88 commented Apr 24, 2023

inside https://github.com/dbt-athena/dbt-athena/blob/main/dbt/include/athena/macros/materializations/models/table/table_hive_ha.sql#L42 please refactor the method to call delete_from_glue_catalog instead of drop!

@nicor88
Copy link
Member

nicor88 commented Apr 25, 2023

@Jrmyy would you mind to have a look at this one?
@chrischin478 so far code looks good to me, did you run integration tests ?

@chrischin478
Copy link
Contributor Author

@Jrmyy would you mind to have a look at this one? @chrischin478 so far code looks good to me, did you run integration tests ?

@nicor88 There's no option for me to reply to this comment for some reason. I ran integration tests and there were a few that skipped but the ones that ran all passed.

@nicor88
Copy link
Member

nicor88 commented Apr 27, 2023

@chrischin478 do you think that you can add some unit tests for delete_from_glue_catalog ? we aim to release tomorrow ideally, and will be nice to have this one in. Thanks:)

README.md Show resolved Hide resolved
dbt/adapters/athena/impl.py Show resolved Hide resolved
tests/unit/test_adapter.py Show resolved Hide resolved
tests/unit/test_adapter.py Show resolved Hide resolved
tests/unit/test_adapter.py Show resolved Hide resolved
README.md Show resolved Hide resolved
tests/unit/test_adapter.py Show resolved Hide resolved
dbt/adapters/athena/impl.py Outdated Show resolved Hide resolved
@Jrmyy Jrmyy modified the milestones: v1.5.0, v1.4.5 May 4, 2023
@Jrmyy Jrmyy requested a review from nicor88 May 4, 2023 07:13
@Jrmyy Jrmyy merged commit 9dfbdef into dbt-athena:main May 4, 2023
7 checks passed
@brabster brabster mentioned this pull request May 8, 2023
@nicor88 nicor88 mentioned this pull request May 23, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants