yyanyy commented on pull request #2214: URL: https://github.com/apache/iceberg/pull/2214#issuecomment-781664020
Thanks Jack for reviewing the PR! > but newDataWriter is not updated. I think `newDataWriter` does have it in the last argument of constructor `DataWriter`? Or did I misunderstand your question? If you were actually referring to `newAppender`, since the creation of `ContentFile` that stores sort order information is outside the responsibility of `FileAppender`, the caller of it that creates data/delete file is responsible for passing the sort order around. I think it's the same reason as why we created `DataWriter`. > the methods you updated are basically not used Yes, in this PR I was trying to add capability of passing sortOrder info to writers within Iceberg library. I did think about expanding the support to `SparkWrite` but eventually felt a bit hesitant to do so, since at the time when I worked on this PR, in Spark community the sort order design was in an early phase, and I wasn't sure if my assumption on how sort order information comes from in `SparkWrite` is accurate, that I'd rather let people who are the expert in individual engines to do this integration with library writers since they have to also integrate with engine anyway. I think the row level delete support is also in this style - `SparkAppenderFactory` constructor could take in row level delete related arguments, but `SparkWrite` and other spark code are not using such constructor yet. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
