-
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
Decouple caching columns from caching table comments in glue for Iceberg #23483
Decouple caching columns from caching table comments in glue for Iceberg #23483
Conversation
8e48623
to
8ec11df
Compare
/test-with-secrets sha=8ec11dff4c3f73b736a6428937659c8fea7e8cfb |
The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/11004474695 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good except for property naming.
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java
Outdated
Show resolved
Hide resolved
ab5c38c
to
d0f9ad0
Compare
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java
Outdated
Show resolved
Hide resolved
d0f9ad0
to
42a96a5
Compare
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java
Outdated
Show resolved
Hide resolved
42a96a5
to
abad951
Compare
What is the user impact of this change that we can add to the release notes. Currently the suggested entry is just talking about implementation details. Something like Ensure table comments are available from Glue even when columns are cached and comment text is long. but less vague maybe? |
Something like: |
Description
Currently, when
iceberg.glue.cache-table-metadata
is set to true, columns and table comment are either stored together in Glue or none of them is stored.The goal of this change is to prevent
TrinoGlueCatalog
from not caching table columns in glue when table comment is too long. It seems wrong that a decision to cache columns depends on the length of the table comment.This commit changes that and makes storing them in Glue independent from each other.
Additional context and related issues
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:
Decouple storing columns definition from storing table comments in glue