AHeise opened a new pull request, #29881:
URL: https://github.com/apache/flink/pull/29881

   ## What is the purpose of the change
   
   A catalog that returns views unresolved (`CatalogView.of(...)` rebuilt from 
stored text, as persistent catalogs do) makes planning exponential in the 
nesting depth of views:
   
   ```sql
   CREATE VIEW v0 AS SELECT * FROM t;
   CREATE VIEW v1 AS SELECT * FROM v0 UNION ALL SELECT * FROM v0;
   -- ... up to v6
   EXPLAIN SELECT * FROM v6;
   ```
   
   `Catalog#getTable` calls for this one `EXPLAIN`:
   
   | Object | Before | After |
   |---|---|---|
   | `v6` | 2 | 2 |
   | `v5` | 8 | 8 |
   | `v4` | 32 | 8 |
   | `v3` | 128 | 8 |
   | `v2` | 512 | 8 |
   | `v1` | 2048 | 8 |
   | `v0` | 8192 | 8 |
   | `t` | 32768 | 8 |
   
   Since FLINK-37098, `CatalogManager#resolveCatalogView` parses and validates 
the view's expanded query to keep its time attributes. Calcite looks up every 
referenced view more than once during that parse, and each lookup resolves the 
view again, recursively.
   
   - Catalogs that store `ResolvedCatalogView`s (e.g. `GenericInMemoryCatalog` 
after `CREATE VIEW`) return early and are not affected.
   
   ## Brief change log
   
   - `CatalogManager` reuses view resolutions while an outermost view 
resolution is in progress, keyed by identifier, expanded query and unresolved 
schema.
   - The cache is dropped when the outermost resolution returns, so no 
resolution outlives the statement that triggered it: views or tables changed 
between statements are always resolved again.
   - Views resolved without an identifier (parsed against the current 
catalog/database) are not cached.
   
   ### Note for reviewers
   
   - The scope is one outermost `resolveCatalogView` call, since 
`CatalogManager` doesn't see statement boundaries. Each top-level lookup of the 
queried view still starts its own resolution (the 2 for `v6` above), which is a 
constant factor.
   - A view whose inner parse fails falls back to `SqlCatalogViewTable` 
expansion, which runs outside the cache. Such statements fail anyway; a 
per-statement scope wrapped around `ParserImpl#parse` would cover both and can 
follow if wanted.
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   
   - 
`NestedViewResolutionTest#testRepeatedlyReferencedViewsAreLookedUpIndependentlyOfDepth`:
 the lookup counts above; fails without the change (`expected: 8 but was: 
8192`).
   - 
`NestedViewResolutionTest#testTimeAttributesArePreservedThroughNestedViews`: a 
window TVF over a view of a view of a rowtime table still plans, i.e. 
FLINK-37098 behavior is kept when a resolution is reused.
   - Existing catalog, view and window planner tests pass.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: no
     - The serializers: no
     - The runtime per-record code paths (performance sensitive): no
     - Anything that affects deployment or recovery: JobManager (and its 
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
     - The S3 file system connector: no
   
   ## Documentation
   
     - Does this pull request introduce a new feature? no
     - If yes, how is the feature documented? not applicable
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes (please specify the tool below)
   
   Generated-by: Claude Code (Claude Opus 5.5)
   


-- 
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]

Reply via email to