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

Postgres normalization #831

Merged
merged 9 commits into from Nov 6, 2020
Merged

Postgres normalization #831

merged 9 commits into from Nov 6, 2020

Conversation

sherifnada
Copy link
Contributor

@sherifnada sherifnada commented Nov 6, 2020

What

  • changes the format of the JSON path in python code to be a list instead of a string
  • adds a macro that formats the JSON path list according to each destination's requirements. Currently supports bigquery and postgres, but should be trivial to add other formatting methods.
  • switch all macros to use the path formatting macro
  • add a gradle dependency for building destination images on the normalization code

Remaining items

  • Find a solution for Postgres not allowing us to replace a table which has dependencies. See PostgresDestination.java#close for context on how we replace tables.

@@ -43,7 +43,7 @@

private static final Logger LOGGER = LoggerFactory.getLogger(DefaultNormalizationRunner.class);

public static final String NORMALIZATION_IMAGE_NAME = "airbyte/normalization:0.1.0";
public static final String NORMALIZATION_IMAGE_NAME = "airbyte/normalization:dev";
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be in some config files or based on env variables so we can switch from dev mode to prod easily or something?

{% macro json_extract(json_column, json_path) -%}
{{ adapter.dispatch('json_extract')(json_column, json_path) }}
{% macro json_extract(json_column, json_path_list) -%}
{{ adapter.dispatch('json_extract')(json_column, format_path(json_path_list)) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the call to the adapter.dispatch function, it shouldn't need to make the call to format_path right?

(you didn't do so on the other variants of the macro either)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, I think it shouldn't need a format_path call

@ChristopheDuong ChristopheDuong merged commit 3a18856 into chris/dbt Nov 6, 2020
@sherifnada sherifnada deleted the sherif/dbt-postgres branch November 6, 2020 18:15
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

2 participants