Make mapping of Trino schema to BigQuery dataset more explicit#19860
Make mapping of Trino schema to BigQuery dataset more explicit#19860wendigo merged 2 commits intotrinodb:masterfrom
Conversation
plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryClient.java
Outdated
Show resolved
Hide resolved
plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryClient.java
Outdated
Show resolved
Hide resolved
plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryClient.java
Outdated
Show resolved
Hide resolved
|
@hashhar can you run this with secrets? |
|
/test-with-secrets sha=dde85321ffea09243d75b078e0798e6b89d9e7d2 |
|
The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/6969121963 |
nineinchnick
force-pushed
the
bigquery-metadata-refactor
branch
from
dde8532 to
6963392
Compare
|
/test-with-secrets sha=696339208803d79d246f771d551d2eece81aea58 |
|
The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/7004090940 |
There was a problem hiding this comment.
I'm a little wondering why we need this method. There are still many DatasetId.getDataset usages.
There was a problem hiding this comment.
There's a single use of this method when we translate BigQuery datasets into Trino schemas. In all other places, we receive a Trino schema and do a reverse translation.
There was a problem hiding this comment.
The project-id is retrieved from client and passed to client.getDataset method. How about changing getDataset's method argument for taking only remoteSchema? Same for toRemoteDataset method.
There was a problem hiding this comment.
The DatasetId uniquely identifies a dataset, so if client.getDataset receive it and use a different project ID (uses only part of the composite key) it would be an incorrect behavior which only coincidentally works because we always use the same project ID here.
There was a problem hiding this comment.
projectId is always the same between if and else, right?
There was a problem hiding this comment.
It doesn't have to be
plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryMetadata.java
Outdated
Show resolved
Hide resolved
This is only a refactor to make it easier to spot when Trino schemas are mapped to BigQuery datasets and the other way around.
nineinchnick
force-pushed
the
bigquery-metadata-refactor
branch
from
6963392 to
b903a2a
Compare
4 participants
Description
When reading through the BigQuery metadata class, it's hard to spot where exactly we map Trino schemas to BigQuery datasets, especially with the code that disambiguates them. This refactor is supposed to make it more explicit.
Additional context and related issues
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: