aokolnychyi commented on code in PR #54488: URL: https://github.com/apache/spark/pull/54488#discussion_r2922962972
########## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala: ########## Review Comment: After this PR is in, can / should we get rid of `writeOptions` in `AppendData`? DSv2 options are handled as options in `DataSourceV2Relation` and `AppendData` only has options for schema evolution, meaning it can be replaced with the flag you are adding here. We can still propagate `mergeSchema` to write builders in connectors. One approach could be to have `writeOptions` as a method that would populate options on demand. That way, the merging of options in `V2Writes` would continue to work but we won't have to populate the options upfront here. ########## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala: ########## Review Comment: It is confusing to have both write option and the flag in this method. -- 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]
