ayushbilala commented on PR #54824: URL: https://github.com/apache/spark/pull/54824#issuecomment-4405723357
@cloud-fan Thanks for the detailed review. On duplication / unified command: I opted to fix the underlying bugs in place rather than introduce a new command class, to keep the PR focused. Both bugs you identified are now fixed: - V2 temp view listing: ShowTablesExec guards listTempViews() with !catalog.isInstanceOf[TableViewCatalog] && !CatalogV2Util.isSessionCatalog(catalog) avoiding duplicates for both session-catalog and TableViewCatalog cases. ShowTablesExtendedExec uses !CatalogV2Util.isSessionCatalog(catalog). - V1 catalog name: ShowTablesCommand now uses sparkSession.sessionState.catalogManager.v2SessionCatalog.name(). If you'd prefer the unified approach, I'll be happy to restructure. On duplicate detection in tests: Added assert(names.distinct.length == names.length, ...) to the two tests most likely to surface duplicates: "SHOW TABLES AS JSON includes temp views" and "SHOW TABLE EXTENDED AS JSON with local temp view". -- 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]
