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 concat macro when fields list is of length 1. #447

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jlkravitz
Copy link

@jlkravitz jlkravitz commented Sep 11, 2023

This pull request fixes the issue described here.

Should this macro handle the case where fields|length == 0, or is that case handled upstream?

@ericmuijsvanoord
Copy link
Contributor

Option: you could also do: CONCAT('', field_list)

@jlkravitz
Copy link
Author

Option: you could also do: CONCAT('', field_list)

I like that @ericmuijsvanoord – updated the PR!

@jlkravitz
Copy link
Author

Hi @ericmuijsvanoord whats the process for getting this merged?

@ericmuijsvanoord
Copy link
Contributor

See this pull request: #461
You need to attach a maintainer which can trigger the tests. However, in the current state, it is best to wait until the pull request 461 is passed. This fully revamps the adapter, based of dbt-fabric.

@astocks
Copy link

astocks commented Apr 23, 2024

@jlkravitz any movement here? This is still causing issues with dbt-utils as you outlined in the OP.

@jlkravitz
Copy link
Author

jlkravitz commented Apr 24, 2024 via email

@ericmuijsvanoord
Copy link
Contributor

I don't think I can help with this, but there are conflicts that have to be resolved first (GitHub message).

@astocks
Copy link

astocks commented Apr 26, 2024

I don't think I can help with this, but there are conflicts that have to be resolved first (GitHub message).

I can try to pick this up and get the tests to pass. Should I open a new PR? @ericmuijsvanoord

@ericmuijsvanoord
Copy link
Contributor

@astocks : im not a maintainer I think. I'm not able to start the tests.
Reach out to one of the maintainers.

@schlich
Copy link
Contributor

schlich commented Aug 12, 2024

If help is still needed on this, seems like there are conflicts with the main branch preventing running the checks

@jlkravitz
Copy link
Author

See this pull request: #461 You need to attach a maintainer which can trigger the tests. However, in the current state, it is best to wait until the pull request 461 is passed. This fully revamps the adapter, based of dbt-fabric.

it seems this file doesn't exist anymore now that this PR has been merged. @schlich Is this still an issue? If not, perhaps we can close the PR. I haven't been using dbt a ton lately so am unable to confirm whether or not this issue still exists.

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.

4 participants