adriangb commented on PR #22372: URL: https://github.com/apache/datafusion/pull/22372#issuecomment-4527318258
Heads-up for reviewers who recall #19137: this PR changes \`with_table_partition_cols\` from **append** back to **replace** semantics. Worth noting the history — append wasn't the longstanding behavior. Before #19137 the setter simply did \`self.table_partition_cols = partition_cols\` (replace). #19137 switched it to append (when it introduced the inner \`Arc\` and, as a side effect, the shared-\`Arc\` panic this PR fixes), framed in its commit message as "fixed a bug when called multiple times." But appending was just *picking a behavior* for repeated calls — it isn't required or more correct than replacing. So this PR restores the original replace behavior, which also matches the usual \`with_*\` builder convention and avoids accidentally duplicating partition columns (the concern @comphead raised above). No production code relied on append; only the unit tests did, and they're updated accordingly. -- 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]
