-
Notifications
You must be signed in to change notification settings - Fork 498
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
union should explicitly specify table name in casts #173
union should explicitly specify table name in casts #173
Comments
I would think that's a general problem with BigQuery and would have to be addressed everywhere you cast a column, not just in this macro? The other obvious "fix" is to not have columns with the same name as a table? |
In my real world, it's a pre-existing table with a single column (plus id)
so changing it will be difficult
I'm not sure I understand why needing to change it elsewhere is an argument
against changing it here?
…On Wed, Nov 6, 2019, 10:39 AM Claus Herther ***@***.***> wrote:
I would think that's a general problem with BigQuery and would have to be
addressed everywhere you cast a column, not just in this macro? The other
obvious "fix" is to not have columns with the same name as a table?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#173>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD3MAANSQZSMQQWOXXUNQLQSLQMPANCNFSM4JJNCLAQ>
.
|
I'll try to send a PR in the next day or two if you'd be willing to accept
it
…On Wed, Nov 6, 2019, 1:40 PM Pete Fein ***@***.***> wrote:
In my real world, it's a pre-existing table with a single column (plus id)
so changing it will be difficult
I'm not sure I understand why needing to change it elsewhere is an
argument against changing it here?
On Wed, Nov 6, 2019, 10:39 AM Claus Herther ***@***.***>
wrote:
> I would think that's a general problem with BigQuery and would have to be
> addressed everywhere you cast a column, not just in this macro? The other
> obvious "fix" is to not have columns with the same name as a table?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#173>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAD3MAANSQZSMQQWOXXUNQLQSLQMPANCNFSM4JJNCLAQ>
> .
>
|
better support for an edge case in Bigquery, which gets confused by casting a column with the same name as its parent table. Fixes dbt-labs#173
I said as much in the linked PR, but to follow up on @clausherther's comment above: a good approach to address this issue on BQ might be to alias the table name instead of changing the source table/column names. I do wish that BigQuery was a little bit smarter here.... alas. So, this works:
and this also works:
but this does not:
All things considered, I think i like the table alias more that the column qualification. It can be really challenging to get quoting/capitalization exactly right on different databases in different contexts. I think it's straightforward here (nice work on that fix @wearpants) but it other contexts, it may be preferable to alias tables instead. |
This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days. |
Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers. |
Hi @drewbanin, I also face this issue in my development with BigQuery, please help me to review the changes if it's make sense to you. |
Bigquery gets confused by tables that have a column with the same name in CASTs, and will try to cast the entire table instead of the column.
I think this is a simple as adding
table.name
here, like:{%- set col_name = adapter.quote(table.name) + '.' + adapter.quote(col_name) if col_name in table_columns[table] else 'null' %}
The text was updated successfully, but these errors were encountered: