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

seed: Add new macro get_csv_sql #5207

Merged
merged 1 commit into from
May 3, 2022

Conversation

adamantike
Copy link
Contributor

@adamantike adamantike commented May 3, 2022

Description

The implementation of this new macro get_csv_sql comes as a suggestion by @jtcohen6 at dbt-labs/dbt-snowflake#112 (comment).

The underlying reason is that the dbt-snowflake package needs to run these queries inside a transaction, but with the current implementation, the fix would require to duplicate the entire default seed materialization logic in the package.

Resolves #5206

Checklist

@adamantike adamantike requested a review from a team May 3, 2022 14:27
@adamantike adamantike requested a review from a team as a code owner May 3, 2022 14:27
@cla-bot cla-bot bot added the cla:yes label May 3, 2022
The implementation of this new macro `get_csv_sql` comes as a suggestion
by @jtcohen6 at
dbt-labs/dbt-snowflake#112 (comment).

The underlying reason is that the `dbt-snowflake` package needs to run
these queries inside a transaction, but with the current implementation,
the fix would require to duplicate the entire default seed
materialization logic in the package.
@adamantike adamantike requested a review from a team as a code owner May 3, 2022 14:30
@jtcohen6
Copy link
Contributor

jtcohen6 commented May 3, 2022

@adamantike Thanks for opening!

I've since realized that I missed an important nuance of the seed materialization in my initial comment over in dbt-labs/dbt-snowflake#112. I'll leave a new comment over there with the update.

@iknox-fa
Copy link
Contributor

iknox-fa commented May 3, 2022

@jtcohen6 I'm not sure I follow the adapters issue here, but the macro part looks sane to me so I'm giving it a 👍, but also adding you as a reviewer to ensure it meets the needs of the adapters ticket.

@iknox-fa iknox-fa requested a review from jtcohen6 May 3, 2022 16:01
@jtcohen6
Copy link
Contributor

jtcohen6 commented May 3, 2022

@iknox-fa The change here is totally fine. It just won't actually address the root cause of the original issue (dbt-labs/dbt-snowflake#112).

@iknox-fa iknox-fa merged commit a4376b9 into dbt-labs:main May 3, 2022
agoblet pushed a commit to BigDataRepublic/dbt-core that referenced this pull request May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-596] [Feature] Allow customization of seed materialization logic for table creation/insertion
3 participants