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

Redshift patch #170

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open

Redshift patch #170

wants to merge 38 commits into from

Conversation

thoffmann-sf
Copy link
Collaborator

@thoffmann-sf thoffmann-sf commented Apr 26, 2024

Added all RedShift datatypes to the Ghost_Records_per_Datatype macro and fixed the other macros that were broken for RedShift and Postgres

@thoffmann-sf thoffmann-sf reopened this Apr 26, 2024
Copy link
Collaborator

@tta-scalefree tta-scalefree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please review macro for ghost_record_per_datatype and align code with other adapters.

{%- elif datatype == 'FLOAT' %} CAST('0' as FLOAT) as {{ alias }}
{%- elif datatype == 'BOOLEAN' %} CAST('FALSE' as BOOLEAN) as {{ alias }}
{%- elif datatype == 'VARBINARY' %} 'NULL'::varbyte as {{ alias }}
{%- elif datatype == 'VARCHAR' or datatype == 'CHARACTER VARYING' or datatype == 'NVARCHAR' or datatype == 'TEXT' %} '{{unknown_value__STRING}}' as {{ alias }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please group the datatypes into array for ease of extending this code in the future, see line 363.
Same for the following code lines.

@tkiehn
Copy link
Collaborator

tkiehn commented May 13, 2024

Hi,

the change from #172 did change the default global variable, which (if copied from the package) is used by all adapters.
I have opened PR #177 that changes the hardcoded default values directly in the macros, which are used if no global variable is specified.

Best Regards,
Theo

thoffmann-sf and others added 5 commits May 13, 2024 12:27
@tkirschke
Copy link
Member

Hi @thoffmann-sf ,
Can you please add all related issues as related items to this PR?

Best regards
Tim

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.

None yet

5 participants