Repository navigation
Include star for ref columns - #193
Conversation
…ata-quality into include-star-for-ref-columns
…ata-quality into include-star-for-ref-columns
…latform/cloud-data-quality into include-star-for-ref-columns � Conflicts: � clouddq/templates/dbt/macros/failed_records_query.sql � clouddq/templates/dbt/macros/macros.sql � tests/resources/test_render_run_dq_main_sql_expected_custom_sql_statement.sql
…ata-quality into include-star-for-ref-columns
| "column_id": self.column_id, | ||
| "reference_columns_id": self.reference_columns_id, | ||
| "include_reference_columns": include_reference_columns, | ||
| "include_all_reference_columns": include_all_reference_columns, |
There was a problem hiding this comment.
@nauh I have added this in configs dictionary to identify the configs for job if it required all columns or no with this flag.
| include_reference_columns = self.resolve_reference_columns_config( | ||
| configs_cache | ||
| ).include_reference_columns | ||
| if include_reference_columns[0] == "*": |
There was a problem hiding this comment.
Can we also check if the length of the include_reference_columns is 1?
There was a problem hiding this comment.
in case we have both * and few other columns in reference columns section, I should raise an error or log a warning and continue with execution with including reference columns.
There was a problem hiding this comment.
I think it's probably better to raise an error if the list include "*" and other columns.
There was a problem hiding this comment.
agree we should check and throw error if user include "*" and other columns
There was a problem hiding this comment.
we need to have 3 conditions: * in include_reference_columns AND length=1, * in include_reference_columns AND length>1, else include columns
| CONTACT_DETAILS_ALL_REFERENCE_COLUMNS: | ||
| include_reference_columns: | ||
| - "*" | ||
|
|
||
| 311_SERVICE_REQUESTS_ALL_REFERENCE_COLUMNS: | ||
| include_reference_columns: | ||
| - "*" |
There was a problem hiding this comment.
should both of these reference IDs just be a single one called "INCLUDE_ALL_REFERENCE_COLUMNS"?
There was a problem hiding this comment.
incorporated
|
looks good thanks @AmandeepSinghCS . I have added a few comments. |
No description provided.