huan233usc commented on PR #14984: URL: https://github.com/apache/iceberg/pull/14984#issuecomment-5350386489
> Thank you @manuzhang for persistence, sorry about the previous review if its not so clear. I'll try to make it more clear in the future. It's just a huge pr so hard to review as other folks mention. > > I'm a bit swamped but try to take a look in next day or two. > > By the way, @nssalian would one option maybe, we merge some version and then fix different parts in smaller prs? For the release, we will have the source code, but this won't make any release artifact, would that work? +1 to merging a version first and then fixing the individual parts in smaller PRs — right now it does feel like we're reviewing a moving target. IIUC about the changes -- the bulk of the real logic is the view migration to Spark 4.2's RelationCatalog (Spark now owns view parsing/resolution, so ResolveViews changes a lot, plus the new Iceberg*V2View exec nodes and catalog wiring). The geo side looks mostly like a mechanical GeometryVal/GeographyVal → BinaryView migration. We could do additional follow-ups items and check like cross-version view testing to guard against regressions. -- 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]
