Skip to content

feat: Add file_schema, cache, and expr_adapter_factory accessors to ListingTable and ListingTableConfig - #27363

Merged
alamb merged 2 commits into
apache:mainfrom
Sruhvx-jpg:feat/listing-table-accessors
Nov 12, 2026
Merged

alamb merged 2 commits into
apache:mainfrom
Sruhvx-jpg:feat/listing-table-accessors

Conversation

@Sruhvx-jpg

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Follow-up to #27317. Closes #27314.

Rationale for this change

In ListingTable and ListingTableConfig, several fields lacked accessors:

  1. ListingTable::file_schema: Currently table.schema() returns the combined table schema (file schema + Hive partition columns). Callers had no way to inspect or retrieve the raw physical file schema directly, requiring manual partition-column filtering workarounds as seen in API to more easily update schema adapter factory on a ListingTable #27314.
  2. ListingTable::cache: with_cache setter existed, but there was no getter to inspect or forward the attached FileStatisticsCache.
  3. ListingTableConfig::expr_adapter_factory: with_expr_adapter_factory setter existed, but the field was pub(crate) without a public getter.

What changes are included in this PR?

  1. Added pub fn file_schema(&self) -> &SchemaRef to ListingTable.
  2. Added pub fn cache(&self) -> Option<&Arc<FileStatisticsCache>> to ListingTable.
  3. Added pub fn expr_adapter_factory(&self) -> Option<&Arc<dyn PhysicalExprAdapterFactory>> to ListingTableConfig.
  4. Added unit tests for all three accessors in table.rs and config.rs.

Are these changes tested?

Yes, tested via unit tests in datafusion-catalog-listing:

  • test_file_schema
  • test_cache
  • test_config_expr_adapter_factory

Verified with cargo fmt and cargo clippy.

@Sruhvx-jpg

Copy link
Copy Markdown
Contributor Author

CC @alamb

@github-actions github-actions Bot added the catalog Related to the catalog crate label Nov 12, 2026

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @Sruhvx-jpg

}

#[cfg(test)]
mod tests {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure we need to add a regression test for an accessor

@Sruhvx-jpg Sruhvx-jpg Nov 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, testing a wrapper function is unneccessary; scraping it

@Sruhvx-jpg
Sruhvx-jpg force-pushed the feat/listing-table-accessors branch from 059ce30 to e90e57c Compare November 12, 2026 16:27
@Sruhvx-jpg

Sruhvx-jpg commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for review @alamb

Your concern for the regression test was right, hence i scrapped the regression test

@alamb
alamb enabled auto-merge November 12, 2026 16:32
@alamb

alamb commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Thanks again

@alamb
alamb added this pull request to the merge queue Nov 12, 2026
Merged via the queue into apache:main with commit 60e3d5c Nov 12, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

catalog Related to the catalog crate v57.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API to more easily update schema adapter factory on a ListingTable

2 participants