-
Notifications
You must be signed in to change notification settings - Fork 3k
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 BigQuery case-insensitive-cache #23481
Conversation
cacc1ed
to
0c40ca2
Compare
/test-with-secrets sha=0c40ca249b2b6874f0f0feaa9c7774a8ad6b03c4 |
The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/10920739075 |
0c40ca2
to
ae6499b
Compare
plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryClient.java
Outdated
Show resolved
Hide resolved
ae6499b
to
847cecb
Compare
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
847cecb
to
ff782a7
Compare
/test-with-secrets sha=ff782a76f6a13e7516f68ed0ec044207d73b53c6 |
The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/10957801373 |
Failed on not related |
Description
Right now
case-insensitive-cache
may contain only one value (in most cases) withprojectId
as key and names mapping as value. This causes cache to be build once and is never updated until it's invalidated. Also it's build with values available from one query only. It's not big problem for schema as for all queries we always supply cache fromclient.listDatasetIds(projectId)
.For table it's buggy as some queries have limited information.
For example:
This PR changes cache structure to be based on table/schema name as key. Also cache is updated on every query if key is not found.
Additional context and related issues
Change in test to remove
.put("bigquery.service-cache-ttl", "0ms")
is needed to get cache working. With this property set to 0.BigQueryClient
is initialised for each query, which results in building cache from scratch for each query.Release notes
( ) 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.
(x) Release notes are required, with the following suggested text: