Skip to content

fix: rank catalog configuration above table properties when constructing FileIO - #3956

Open
YuangGao wants to merge 2 commits into
apache:mainfrom
YuangGao:fix-3931-catalog-config-precedence
Open

YuangGao wants to merge 2 commits into
apache:mainfrom
YuangGao:fix-3931-catalog-config-precedence

Conversation

@YuangGao

@YuangGao YuangGao commented Sep 12, 2026

Copy link
Copy Markdown

Closes #3931

Rationale for this change

  • Catalog._load_file_io computed load_file_io({**self.properties, **properties}, location) where properties is metadata.properties — table metadata outranked the catalog configuration.
  • An operator sets catalog properties; anyone who can commit to a table sets table properties. The lower-trust source won.
  • Reachable keys include implementation selection (py-io-impl, s3.retry-strategy-impl — dotted paths passed to importlib.import_module) and transport configuration (s3.endpoint, s3.proxy-uri, s3.signer.uri, gcs.service.host, the ADLS authorities).
  • Table properties now rank lowest, still supplying keys the catalog leaves unset.
  • REST precedence becomes table metadata < catalog < server config < vended credentials, so remote signing and credential vending are unchanged.

Are these changes tested?

  • test_sql.py — covers Catalog._load_file_io through create_table / load_table.
  • test_rest.py — covers RestCatalog._load_file_io, and pins server config above the catalog.

Are there any user-facing changes?

  • A table property naming a FileIO key the catalog already sets no longer takes effect.
  • Keys the catalog does not set still come from the table.

@YuangGao YuangGao changed the title Rank catalog configuration above table properties when constructing FileIO fix: rank catalog configuration above table properties when constructing FileIO Sep 12, 2026
@YuangGao
YuangGao marked this pull request as ready for review September 12, 2026 05:30

@rambleraptor rambleraptor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this looks good. I spent way too much time trying to trace out the ordering (we have too many properties related variables, naming is hard)

identifier=("static-table", metadata_location),
metadata_location=metadata_location,
metadata=metadata,
io=load_file_io({**properties, **metadata.properties}, location=metadata_location),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is untested.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

added a test

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.

Table properties override catalog configuration when constructing FileIO

2 participants