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

fix: View Creation fails with with reserved words in project name #22

Merged
merged 1 commit into from
Apr 2, 2023
Merged

fix: View Creation fails with with reserved words in project name #22

merged 1 commit into from
Apr 2, 2023

Conversation

chollinger93
Copy link
Contributor

tl;dr

Once a reserved word, such as new, exists as part of a view creation step (e.g., in the project or table name), the BQ query fails.

column_name_transforms.quote does not work to mitigate this (only quotes field names).

CREATE OR REPLACE VIEW new-XXX.XXX.repositories_view AS 
-- ...
google.api_core.exceptions.BadRequest: 400 Syntax error: Unexpected keyword NEW at [1:24] 

You can try this in the BQ consoled:

CREATE TABLE new-project.test -- ...
CREATE TABLE create.tablename -- implicit project
-- Syntax error: Unexpected keyword NEW at [1:14] 

Steps to reproduce

python3 -m pip install --user pipx
pipx install meltano
meltano init
meltano add extractor tap-github
meltano add loader target-bigquery --variant z3z1ma

Failure Scenarios

Denormalized, Quoted

The following statement was meant to achieve the creation of a regular, flat table. That failed with a different error, but I create a separate ticket for that.

# ...
plugins:
  extractors:
  - name: tap-github
    variant: meltanolabs
    pip_url: git+https://github.com/MeltanoLabs/tap-github.git
    config:
      start_date: '2023-01-01T00:00:00Z'
      searches:
      - name: Repos
        query: Meltano/*
    select: [repositories.*]
  loaders:
  - name: target-bigquery
    variant: z3z1ma
    pip_url: git+https://github.com/z3z1ma/target-bigquery.git
    config:
      project: new-XX
      location: XX
      dataset: XX
      credentials_path: $MELTANO_PROJECT_ROOT/client_secrets.json
      denormalized: false
      generate_view: true
      column_name_transforms:
        quote: true

Results in

google.api_core.exceptions.BadRequest: 400 Syntax error: Unexpected keyword NEW at [1:24] 

Caused by the generated statement:

CREATE OR REPLACE VIEW new-XXX.XXX.repositories_view AS 
-- ...

The word "new" only shows up in the project name as prefix, as outlined in the statement above, but causes the CTAS to fail. This is a legal project name, mind you.

Default Settings

Works, but neither denormalizes, nor creates a view, making this data hard to use.

Suggested Fix: View Creation

Since it's legal BQ SQL to escape the entire table identifier, we could do that in the SQL strings.

This PR makes this a function of the BigQueryTable model, which is cleaner, IMO. name or __str__ should not return a quoted string.

Caveats:

  • Since this wasn't a function of the model before, it's hard for me to double check that I covered this in all cases.
  • The resulting view has a Array cannot have a null element; error in writing field topics error, but that could not be caused by this PR. That should probably be fixed, though. Probably also a separate ticket.

All UTs and ITs pass, though.

Re-Test

denormalized generate view quote result
FALSE TRUE FALSE works
FALSE TRUE TRUE works

@z3z1ma
Copy link
Owner

z3z1ma commented Apr 2, 2023

LGTM, thanks for the contribution 🔥

Look forward to digging into the other issue you mentioned.

Array cannot have a null element

https://github.com/z3z1ma/target-bigquery/blob/main/target_bigquery/core.py#L783
Will probably involve some munging here to handle the fact an array may have NULLs. We should also consider an escape hatch that uses some sort of pattern (probably the same as what Meltano select rules use) to override the casting for a specific datum.

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.

2 participants