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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

馃帀 Base Normalization: quote schema name to allow reserved keywords #14683

Merged
merged 14 commits into from Jul 27, 2022
Merged

馃帀 Base Normalization: quote schema name to allow reserved keywords #14683

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
907bbe4
quote this.schema
grubberr Jul 13, 2022
f114106
basic-normalization.md updated
grubberr Jul 13, 2022
759ea2e
NormalizationRunnerFactory.java updated
grubberr Jul 13, 2022
546bef5
Merge branch 'master' into grubberr/14674-base-normalization
grubberr Jul 14, 2022
cc87c28
format code
grubberr Jul 14, 2022
0bcd4b7
Merge branch 'master' into grubberr/14674-base-normalization
grubberr Jul 20, 2022
1a752fe
Merge branch 'master' into grubberr/14674-base-normalization
grubberr Jul 24, 2022
392863d
lint code
grubberr Jul 24, 2022
743163d
lint code (2)
grubberr Jul 24, 2022
c2bfd09
Merge branch 'master' into grubberr/14674-base-normalization
grubberr Jul 24, 2022
7fed9d1
Merge branch 'master' into grubberr/14674-base-normalization
grubberr Jul 27, 2022
755598a
basic-normalization.md updated
grubberr Jul 27, 2022
ab4a8aa
Merge branch 'master' into grubberr/14674-base-normalization
grubberr Jul 27, 2022
675248f
NormalizationRunnerFactory.java updated
grubberr Jul 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion airbyte-integrations/bases/base-normalization/Dockerfile
View file
Open in desktop
Expand Up @@ -28,5 +28,5 @@ WORKDIR /airbyte
ENV AIRBYTE_ENTRYPOINT "/airbyte/entrypoint.sh"
ENTRYPOINT ["/airbyte/entrypoint.sh"]

LABEL io.airbyte.version=0.2.8
LABEL io.airbyte.version=0.2.9
LABEL io.airbyte.name=airbyte/normalization
10 changes: 6 additions & 4 deletions .../bases/base-normalization/normalization/transform_catalog/destination_name_transformer.py
View file
Open in desktop
Expand Up @@ -181,13 +181,15 @@ def __normalize_identifier_name(
return f"'{result}'"
return result

def apply_quote(self, input: str) -> str:
def apply_quote(self, input: str, literal=True) -> str:
if literal:
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure I understand this change - previously all of our quotes were using quote or adapter.quote; why do we need to switch them to just wrapping in single-quote marks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes I agree not very obvious

before my change this function generated only such output:

adapter.quote('table_name')

as you know on jinja2 "level" it means: pass String literal 'table_name' to jinaj2 function adapter.quote

but I have a new requirement I need to get:

adapter.quote(this.schema)

on jinja2 level it means pass jinja2 variable to jinja2 function.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah that makes sense (also I misread the code, thought it was doing a return f"'{input}'"). So basically this is behaving exactly the same for literal=False.

input = f"'{input}'"
if self.destination_type == DestinationType.ORACLE:
# Oracle dbt lib doesn't implemented adapter quote yet.
return f"quote('{input}')"
edgao marked this conversation as resolved.
Show resolved Hide resolved
return f"quote({input})"
elif self.destination_type == DestinationType.CLICKHOUSE:
return f"quote('{input}')"
return f"adapter.quote('{input}')"
return f"quote({input})"
return f"adapter.quote({input})"

def __normalize_naming_conventions(self, input_name: str, is_column: bool = False) -> str:
result = input_name
Expand Down
4 changes: 3 additions & 1 deletion ...integrations/bases/base-normalization/normalization/transform_catalog/stream_processor.py
View file
Open in desktop
Expand Up @@ -1209,7 +1209,9 @@ def add_to_outputs(
quoted_unique_key=self.get_unique_key(in_jinja=True),
active_row_column_name=active_row_column_name,
normalized_at_incremental_clause=self.get_incremental_clause_for_column(
"this.schema + '.' + " + self.name_transformer.apply_quote(final_table_name),
self.name_transformer.apply_quote("this.schema", literal=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: please use f-strings instead of concatenation. It makes code more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have improved readability with string formatting

+ " + '.' + "
+ self.name_transformer.apply_quote(final_table_name),
self.get_normalized_at(in_jinja=True),
),
unique_key_reference=unique_key_reference,
Expand Down
2 changes: 1 addition & 1 deletion ...te-workers/src/main/java/io/airbyte/workers/normalization/NormalizationRunnerFactory.java
View file
Open in desktop
Expand Up @@ -14,7 +14,7 @@
public class NormalizationRunnerFactory {

public static final String BASE_NORMALIZATION_IMAGE_NAME = "airbyte/normalization";
public static final String NORMALIZATION_VERSION = "0.2.8";
public static final String NORMALIZATION_VERSION = "0.2.9";

static final Map<String, ImmutablePair<String, DefaultNormalizationRunner.DestinationType>> NORMALIZATION_MAPPING =
ImmutableMap.<String, ImmutablePair<String, DefaultNormalizationRunner.DestinationType>>builder()
Expand Down
1 change: 1 addition & 0 deletions docs/understanding-airbyte/basic-normalization.md
View file
Open in desktop
Expand Up @@ -353,6 +353,7 @@ Therefore, in order to "upgrade" to the desired normalization version, you need

| Airbyte Version | Normalization Version | Date | Pull Request | Subject |
|:----------------| :--- | :--- | :--- | :--- |
| | 0.2.9 | 2022-07-14 | [\#14683](https://github.com/airbytehq/airbyte/pull/14683) | Quote schema name to allow reserved keywords |
| | 0.2.8 | 2022-07-13 | [\#14522](https://github.com/airbytehq/airbyte/pull/14522) | BigQuery replaces `NULL` array entries with the string value `"NULL"` |
| | 0.2.7 | 2022-07-05 | [\#11694](https://github.com/airbytehq/airbyte/pull/11694) | Do not return NULL for MySQL column values > 512 chars |
| | 0.2.6 | 2022-06-16 | [\#13894](https://github.com/airbytehq/airbyte/pull/13894) | Fix incorrect jinja2 macro `json_extract_array` call |
Expand Down