search

dbt-labs / dbt-external-tables

dbt macros to stage external sources
https://hub.getdbt.com/dbt-labs/dbt_external_tables/latest/
Apache License 2.0
313 stars 123 forks source link

added ignore_case for Snowflake #308

Closed cakkinep closed 3 weeks ago

cakkinep commented 4 months ago

Description & motivation

Snowflake external tables by default do not ignore case of columns of underlying external File, this poses a challenge when defining to exactly define the column names to be able to successfully create external tables and have the column values populated.

This PR adds the capability to use GET_IGNORE_CASE function to lookup the column value instead of standard value: reference.

Checklist

cakkinep commented 3 months ago

@dataders need you help with this integration test failure, its not related to any of the changes i made.

jeremyyeo commented 3 months ago

@cakkinep - I'll have a look to see what's up with those CI errors.

jeremyyeo commented 3 months ago

Some reason some of those external tables (like PEOPLE_JSON_PARTITIONED) had been recreated as tables from some other process perhaps - can't say for sure. Will drop them and rerun ci.

cakkinep commented 3 months ago

Yeah, saw that in the errors, couldn't do much as I didn't have access to the CI environment and resources it is running against. Thanks for working on the fix.

cakkinep commented 3 months ago

@robby-rob can you please check if this PR will solve for what you were looking for in #289 ?

cakkinep commented 3 months ago

@jeremyyeo , @dataders , tagging you both for a review to get this merged. Please let me know if i may have missed anything .

cakkinep commented 3 months ago

@jeremyyeo please let me know if you are not the right person to request a review and get this MR move forward.

robby-rob-slalom commented 3 months ago

@robby-rob can you please check if this PR will solve for what you were looking for in #289 ?

This appears to solve what I was working on. Is there a case for allowing the ignore_case option when not using the infer_schema option? I believe I added that in when #257 was merged, but not sure how important it is.

cakkinep commented 3 months ago

This appears to solve what I was working on. Is there a case for allowing the ignore_case option when not using the infer_schema option? I believe I added that in when #257 was merged, but not sure how important it is.

@robby-rob-slalom This change handles ignore case for both infer_schema and non infer case too.

Thanks for your feedback.

cakkinep commented 2 months ago

@dataders , @jeremyyeo , can you please look into this or get attention of someone who can look into merging this MR?

cakkinep commented 2 months ago

@jeremyyeo , @dataders , can you please review and get this merged?

cakkinep commented 2 months ago

@jeremyyeo , @dataders if you are not the right people to tag for getting this reviewed and merged, please tag those who are responsible.

cakkinep commented 2 months ago

How can we get this merged? @dataders @jeremyyeo

cakkinep commented 1 month ago

@jeremyyeo can you please review and let me know if anything else needs to be done to get this merged?