cloud-fan opened a new pull request, #54247: URL: https://github.com/apache/spark/pull/54247
### What changes were proposed in this pull request? This is a followup to #53788 which moved the namespace length check from individual command handlers to the `CatalogAndIdentifier` extractor. That approach is too aggressive: users can extend the session catalog via `CatalogExtension` and support multi-part namespaces through v2 APIs. The check should only happen when we actually create v1 identifiers like `TableIdentifier`, not at the shared name-resolution layer. ### Changes - **Remove the namespace check from `CatalogAndIdentifier.unapply`** so it remains a pure name-resolution mechanism, preserving `CatalogExtension` flexibility. - **Tighten `CatalogV2Implicits.IdentifierHelper.asTableIdentifier` and `asFunctionIdentifier`** to require exactly one namespace part and throw `REQUIRES_SINGLE_PART_NAMESPACE` (instead of the less precise `IDENTIFIER_TOO_MANY_NAME_PARTS`). This centralizes the validation. - **Remove the now-redundant `V2SessionCatalog.TableIdentifierHelper`** and use the unified `CatalogV2Implicits` conversion everywhere. - **Simplify `ResolveSessionCatalog` extractors** (`ResolvedV1Identifier`, `ResolvedIdentifierInSessionCatalog`, `ResolvedViewIdentifier`) to delegate to `ident.asTableIdentifier`. - **Fix `SparkSqlParser` temp view creation** to check name length before calling `asTableIdentifier`, so the user always sees `notAllowedToAddDBPrefixForTempViewError` instead of a generic error. ### Why are the changes needed? `CatalogExtension` allows users to extend the built-in session catalog and potentially support multi-part namespaces for v2 operations. The early check in `CatalogAndIdentifier` would block such extensions. The namespace length should only be validated when we actually need to create v1 identifiers (e.g. `TableIdentifier`), which inherently require a single-part namespace. Additionally, this PR unifies the scattered namespace validation into a single point (`CatalogV2Implicits.IdentifierHelper`), reducing code duplication and ensuring consistent `REQUIRES_SINGLE_PART_NAMESPACE` errors. ### Does this PR introduce _any_ user-facing change? Yes. Multi-part namespace identifiers now flow through `CatalogAndIdentifier` without error, allowing `CatalogExtension` implementations to handle them. The error is only thrown when the session catalog actually needs to convert to a v1 identifier. ### How was this patch tested? Existing tests updated: - `LookupCatalogSuite` — removed the early-rejection test, added multi-part namespace cases back. - `V2SessionCatalogSuite` — updated to expect `REQUIRES_SINGLE_PART_NAMESPACE`. - `DataSourceV2SQLSuiteV1Filter` and `DataSourceV2FunctionSuite` — all 313 tests pass. ### Was this patch authored or co-authored using generative AI tooling? Yes. Made with [Cursor](https://cursor.com) -- 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]
