mbutrovich commented on PR #22026: URL: https://github.com/apache/datafusion/pull/22026#issuecomment-4390174610
Following up on this: I added `debug_assert!` guards on `TableSchema::with_virtual_columns` (rejects a virtual name that matches a file, partition, or already-registered virtual column) and on `TableSchema::with_table_partition_cols` (rejects a partition name that matches an existing virtual column, so the invariant holds regardless of builder call order). Release builds pay no validation cost, matching the partition-column convention I described above: upstream avoids bad names, core structs stay cheap, and mistakes surface loudly in dev and CI. Four `#[should_panic]` tests cover the collision shapes (virtual-vs-file, virtual-vs-partition, virtual-vs-virtual, and partition-added-after-colliding-virtual). File-vs-partition collisions in `TableSchema` are still unchecked, matching `ListingTable::try_new` and Arrow's `SchemaBuilder::push`. Happy to tighten that in a separate PR if we want to revisit the convention more broadly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
