Fix inconsistent schema projection in ListingTable even when schema is specified #16305

kosiew · 2025-06-06T14:00:02Z

Which issue does this PR close?

Closes Inconsistent schema coercion in ListingTableConfig #16270

Rationale for this change

Understanding the origin of a schema (whether it was inferred or explicitly specified) is important for debugging, reproducibility, and behavioral consistency in systems like DataFusion that operate on dynamic data sources. Previously, this information was not available in the ListingTable or its configuration, making it hard to reason about schema behavior.

What changes are included in this PR?

Schema-source metadata
- Introduced a new SchemaSource enum to track the origin of a schema (None, Inferred, Specified).
- Extended ListingTableConfig and ListingTable to carry and expose this metadata.
- Ensured that schema inference logic respects an explicitly set schema and does not overwrite it.
- Added public accessors (schema_source()) to inspect schema origin in both config and table.
Imports cleanup
- Reorganized imports in table.rs for clarity and consistency.
Test refactoring & additions
- Refactored single-file scan/statistics tests:
  - Kept and cleaned up read_single_file.
  - Unified Parquet-stats coverage into a single test_table_stats_behaviors.
- Consolidated file-listing tests into a parameterized test_list_files_configurations.
- Parameterized insert-into append tests via test_insert_into_parameterized.
- Added comprehensive unit tests for all SchemaSource cases:
  - test_schema_source_tracking_comprehensive
  - infer_preserves_provided_schema
- Removed dozens of redundant individual tests in favor of DRY loops and shared helpers (create_test_schema, generate_test_files…).

Deleted tests → New tests mapping

Deleted test(s)	Replacement tests
`read_single_file` (old)	`read_single_file` (refactored)
`do_not_load_table_stats_by_default` `load_table_stats_when_no_stats`	`test_table_stats_behaviors`
`test_assert_list_files_for_scan_grouping` `test_assert_list_files_for_multi_path` `test_assert_list_files_for_exact_paths`	`test_list_files_configurations`
`test_insert_into_append_new_json_files` `test_insert_into_append_new_csv_files` `test_insert_into_append_2_new_parquet_files_defaults` `test_insert_into_append_1_new_parquet_files_defaults`	`test_insert_into_parameterized`

Are these changes tested?

Yes. This PR includes a full suite of unit tests covering:
- Single-file + stats: read_single_file, test_table_stats_behaviors
- Schema-source tracking: test_schema_source_tracking_comprehensive, infer_preserves_provided_schema
- File-listing grouping: test_list_files_configurations
- Insert-into append behavior: test_insert_into_parameterized

Shared helpers and parameterized loops ensure that every previously tested scenario is still exercised, with improved maintainability and coverage.

Are there any user-facing changes?

Yes:

ListingTable now exposes a schema_source() method, enabling downstream consumers to programmatically check the origin of the schema.
This may help users understand or debug unexpected schema-related behavior when working with listing tables.

There are no breaking changes to the public API, but the enhancement provides improved introspection capabilities.

…TableConfig

…le files in ListingTableConfig

…lapping tests

…ingTable

…port

…lexibility

…ssertion

…stics assertion from Test 1

kosiew · 2025-06-08T03:04:21Z

datafusion/core/src/datasource/listing/table.rs

+        let opt_enabled = ListingOptions::new(Arc::new(ParquetFormat::default()))
+            .with_collect_stat(true);
+        let schema_enabled = opt_enabled.infer_schema(&state, &table_path).await?;
+        let config_enabled = ListingTableConfig::new(table_path)
+            .with_listing_options(opt_enabled)
+            .with_schema(schema_enabled);
+        let table_enabled = ListingTable::try_new(config_enabled)?;
+
+        let exec_enabled = table_enabled.scan(&state, None, &[], None).await?;
+        assert_eq!(
+            exec_enabled.partition_statistics(None)?.num_rows,
+            Precision::Exact(8)
+        );
+        // TODO correct byte size: https://github.com/apache/datafusion/issues/14936
+        assert_eq!(
+            exec_enabled.partition_statistics(None)?.total_byte_size,
+            Precision::Exact(671)
+        );


from previous do_not_load_table_stats_by_default

let opt = ListingOptions::new(Arc::new(ParquetFormat::default())) .with_collect_stat(true); let schema = opt.infer_schema(&state, &table_path).await?; let config = ListingTableConfig::new(table_path) .with_listing_options(opt) .with_schema(schema); let table = ListingTable::try_new(config)?; let exec = table.scan(&state, None, &[], None).await?; assert_eq!( exec.partition_statistics(None)?.num_rows, Precision::Exact(8) ); // TODO correct byte size: https://github.com/apache/datafusion/issues/14936 assert_eq!( exec.partition_statistics(None)?.total_byte_size, Precision::Exact(671) );

kosiew · 2025-06-08T03:10:44Z

datafusion/core/src/datasource/listing/table.rs

+        let opt_default = ListingOptions::new(Arc::new(ParquetFormat::default()));
+        let schema_default = opt_default.infer_schema(&state, &table_path).await?;
+        let config_default = ListingTableConfig::new(table_path.clone())
+            .with_listing_options(opt_default)
+            .with_schema(schema_default);
+        let table_default = ListingTable::try_new(config_default)?;
+
+        let exec_default = table_default.scan(&state, None, &[], None).await?;
+        assert_eq!(
+            exec_default.partition_statistics(None)?.num_rows,
+            Precision::Absent
+        );
+
+        // TODO correct byte size: https://github.com/apache/datafusion/issues/14936
+        assert_eq!(
+            exec_default.partition_statistics(None)?.total_byte_size,
+            Precision::Absent
+        );


Replace this excerpt from previous do_not_load_table_stats_by_default

let opt = ListingOptions::new(Arc::new(ParquetFormat::default())); let schema = opt.infer_schema(&state, &table_path).await?; let config = ListingTableConfig::new(table_path.clone()) .with_listing_options(opt) .with_schema(schema); let table = ListingTable::try_new(config)?; let exec = table.scan(&state, None, &[], None).await?; assert_eq!(exec.partition_statistics(None)?.num_rows, Precision::Absent); // TODO correct byte size: https://github.com/apache/datafusion/issues/14936 assert_eq!( exec.partition_statistics(None)?.total_byte_size, Precision::Absent );

xudong963 · 2025-06-09T09:55:06Z

datafusion/core/src/datasource/listing/table.rs

+/// Indicates the source of the schema for a [`ListingTable`]
+/// PartialEq required for assert_eq! in tests
+#[derive(Debug, Clone, PartialEq)]
+pub enum SchemaSource {


I really like this, the explicit representation definitely will reduce confusion

100% -- this is great

cc @adriangb

xudong963

LGTM, thank you @kosiew / cc @alamb

kosiew · 2025-06-09T11:48:01Z

Thanks @xudong963 for the review.

alamb

Thanks @kosiew and @xudong963 -- I agree this is a really nice improvement

alamb · 2025-06-09T12:37:48Z

datafusion/core/src/datasource/listing/table.rs

+/// Indicates the source of the schema for a [`ListingTable`]
+/// PartialEq required for assert_eq! in tests
+#[derive(Debug, Clone, PartialEq)]
+pub enum SchemaSource {


100% -- this is great

cc @adriangb

datafusion/core/src/datasource/listing/table.rs

alamb · 2025-06-09T12:45:04Z

datafusion/core/src/datasource/listing/table.rs

    /// See documentation and example on [`ListingTable`] and [`ListingTableConfig`]
    pub fn try_new(config: ListingTableConfig) -> Result<Self> {
+        // Extract schema_source before moving other parts of the config
+        let schema_source = config.schema_source().clone();


Minor nit: since SchemaSource is just a bare enum, it might also make sense to #derive(Copy) so we didn't have to clone it explicitly.

This is totally unecessary I just wanted to mention it as a possibility

make sense to #derive(Copy) so we didn't have to clone it explicitly.

💯 and implemented here.

alamb

Thanks @kosiew and @xudong963 -- I agree this is a really nice improvement

…nfig

kosiew added 5 commits June 6, 2025 19:29

feat: Enhance schema inference to preserve provided schema in Listing…

96ea5ef

…TableConfig

feat: Add SchemaSource enum to track schema origin in ListingTableConfig

d5fd7f8

feat: Add schema_source field to ListingTable to track schema derivation

4512ea7

feat: Add tests for schema source tracking in ListingTableConfig

a0ed876

feat: Add tests for inferred and specified schema handling for multip…

82d1bff

…le files in ListingTableConfig

github-actions bot added the core Core DataFusion crate label Jun 6, 2025

kosiew marked this pull request as draft June 6, 2025 14:15

kosiew added 13 commits June 7, 2025 10:19

Merge branch 'main' into list-table-config-file-schema-16270

7787863

Fix tests

d8d86d8

refactor: remove PartialEq from SchemaSource enum

7957b87

docs: add back parameter description for collect_stats in ListingTable

47514f4

test: refactor schema creation in tests to use a helper function

5e3ac75

test: add comprehensive schema source tracking tests and replace over…

dd607e4

…lapping tests

refactor: remove outdated comments and consolidate test cases in List…

29fe6e9

…ingTable

fix: add PartialEq derivation to SchemaSource enum for comparison sup…

517818b

…port

fix: update SQL query in tests to reference correct table name t

feb2f09

refactor: reorganize imports for clarity and consistency in table.rs

46c2646

test: add helper functions to generate test file paths for improved f…

a69642a

…lexibility

Merge branch 'main' into list-table-config-file-schema-16270

43c5972

refactor: reorganize test imports for improved clarity and consistency

890b5e2

kosiew marked this pull request as ready for review June 7, 2025 08:14

kosiew added 3 commits June 7, 2025 17:07

fix clippy errors

712c572

test: add TODO comment to correct byte size in partition statistics a…

22d38c0

…ssertion

test: remove TODO comment for byte size correction in partition stati…

a40a448

…stics assertion from Test 1

kosiew force-pushed the list-table-config-file-schema-16270 branch from 47fcdae to a40a448 Compare June 7, 2025 16:05

kosiew commented Jun 8, 2025

View reviewed changes

Move todo comment to case 1

89923f0

kosiew commented Jun 8, 2025

View reviewed changes

Merge branch 'main' into list-table-config-file-schema-16270

2da395c

xudong963 self-requested a review June 9, 2025 09:54

xudong963 reviewed Jun 9, 2025

View reviewed changes

xudong963 approved these changes Jun 9, 2025

View reviewed changes

kosiew changed the title ~~Fix inconsistent schema projection in ListingTable when file order varies by tracking schema source~~ Fix inconsistent schema projection in ListingTable even when schema is specified Jun 9, 2025

Merge branch 'main' into list-table-config-file-schema-16270

919f323

alamb approved these changes Jun 9, 2025

View reviewed changes

fix: simplify schema source access in ListingTable and ListingTableCo…

21ad8fb

…nfig

kosiew force-pushed the list-table-config-file-schema-16270 branch from 0790f53 to 21ad8fb Compare June 9, 2025 13:43

Fix clippy error

5485976

xudong963 merged commit 2986415 into apache:main Jun 10, 2025
28 checks passed

kosiew deleted the list-table-config-file-schema-16270 branch July 16, 2025 03:21

Fix inconsistent schema projection in ListingTable even when schema is specified #16305

Fix inconsistent schema projection in ListingTable even when schema is specified #16305

Uh oh!

Conversation

kosiew commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Deleted tests → New tests mapping

Are these changes tested?

Are there any user-facing changes?

Uh oh!

kosiew Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

kosiew Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

xudong963 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

alamb Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

xudong963 left a comment

Choose a reason for hiding this comment

Uh oh!

kosiew commented Jun 9, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alamb Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

kosiew Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kosiew commented Jun 6, 2025 •

edited

Loading