Repository: dbt-athena/dbt-athena
Pull request #76 (2022-12-04 21:35:12)

Fix deletion of tables with same prefix location
### Description

There's a bug that would delete a table together with all tables that share the same s3 `external_location` as prefix.

Example:
- table_a lives at s3://mybucket/foo
- table_b lives at s3://mybucket/foo_bar

dbt-athena will delete everything under s3://mybucket/foo, which includes data for table_b (unintentionally, table would be empty afterwards)

The fix is to append a trailing slash '/' to the s3 path, so that only the right data gets deleted (in above example everything under `s3://mybucket/foo/`, which now does **not** include `s3://mybucket/foo_bar`


## Checklist
- [x] You followed [contributing section](https://github.com/dbt-athena/dbt-athena#contributing)
- [x] You added unit testing when necessary
- [x] You added functional testing when necessary

nicor88: @juliansteger-sc I don't consider this a bug, as external location should be set in a way that the path is unique and not shared across tables, for me this specific edge case is rather a bad configuration on the models.
juliansteger-sc: @nicor88 not sure if I made the example clear. I try to pick another one.
when we have parquet files for table `customer`
`s3://cleaned-bucket/customer/file1.parquet ... s3://cleaned-bucket/customer/filen.parquet`
and for table `customer_order`
`s3://cleaned-bucket/customer_order/file1.parquet ... s3://cleaned-bucket/customer_order/filem.parquet`

refreshing the model for customer will also truncate `customer_order`. This is definitely not what we want.

Don't you consider this s3 layout valid? The path to the tables is unique (they live in different 'folders' in the bucket).
We always put our files in s3 locations according to `bucket/schema/table_name`
and some table names are simply prefixes of others.
nicor88: ah I see there is a collision on the prefix, as this prefix is used in another table. Understood, thanks for the explanation, changes looks good to me.

Patch:
diff --git a/dbt/adapters/athena/impl.py b/dbt/adapters/athena/impl.py
index 32105b7..1189d18 100755
--- a/dbt/adapters/athena/impl.py
+++ b/dbt/adapters/athena/impl.py
@@ -145,12 +145,12 @@ class AthenaAdapter(SQLAdapter):
                 return
 
         if table is not None:
-            logger.debug(f"Deleting table data from '{table['Table']['StorageDescriptor']['Location']}'")
             p = re.compile("s3://([^/]*)/(.*)")
             m = p.match(table["Table"]["StorageDescriptor"]["Location"])
             if m is not None:
                 bucket_name = m.group(1)
-                prefix = m.group(2)
+                prefix = m.group(2).rstrip("/") + "/"
+                logger.debug(f"Deleting table data from 's3://{bucket_name}/{prefix}'")
                 s3_resource = client.session.resource("s3", region_name=client.region_name, config=get_boto3_config())
                 s3_bucket = s3_resource.Bucket(bucket_name)
                 s3_bucket.objects.filter(Prefix=prefix).delete()


Test patch:
diff --git a/tests/unit/test_adapter.py b/tests/unit/test_adapter.py
index 9bbc45e..5f7ac4a 100644
--- a/tests/unit/test_adapter.py
+++ b/tests/unit/test_adapter.py
@@ -279,7 +279,7 @@ class TestAthenaAdapter:
         self.mock_aws_service.add_data_in_table("table")
         self.adapter.acquire_connection("dummy")
         self.adapter.clean_up_table(DATABASE_NAME, "table")
-        assert "Deleting table data from 's3://test-dbt-athena-test-delete-partitions/tables/table'" in caplog.text
+        assert "Deleting table data from 's3://test-dbt-athena-test-delete-partitions/tables/table/'" in caplog.text
         s3 = boto3.client("s3", region_name=AWS_REGION)
         objs = s3.list_objects_v2(Bucket=BUCKET)
         assert objs["KeyCount"] == 0