Vamsi-klu commented on PR #58037: URL: https://github.com/apache/spark/pull/58037#issuecomment-5333099923
> **1 addressed, 0 remaining, 1 new to this AI review.** (0 newly introduced, 1 late catch, 0 previously raised.) > **0 blocking, 0 non-blocking, 1 nit.** > The earlier correctness issue is fixed; only a small prose correction remains. > > **Nits**: 1 minor item (see inline comments). > > ## Verification > I traced the V1 table path through `ResolveSessionCatalog`: `external || location.isDefined` selects `CatalogTableType.EXTERNAL`, `LOCATION` populates `storage.locationUri`, and `SessionCatalog.createTable` rejects external tables without that location. The catalog drop path deletes data only for managed tables, so the revised description is consistent with the implementation. > > ## PR metadata suggestions > - Remove the stale claims that an equivalent path option is accepted and that the PR includes an `OPTIONS (PATH ...)` example; the current revision intentionally documents `LOCATION` only. > > <!-- ai-auto-review-authorized head=e91e0aae595321d464b7265efce3e30133289ff2 --> > > <!-- spark-dev-review-delivery:v1 sha256=2b57af1690e67f474d49c28fd001c039e132f3b5055ab04f002c11a8099fdba7 --> > > <!-- ai-code-review --> Resolved, thanks! -- 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]
