Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let SchemaImpl and TableImpl avoid traversal and replacement recursion #15046

Closed
lukaseder opened this issue May 8, 2023 · 2 comments
Closed

Comments

@lukaseder
Copy link
Member

One of the open questions of the new query object model was whether object references like TableField should allow for recursing traversal and replacement into their parent objects:

I.e. a TableField is a Table and a Field, whereas a Table is a Schema and a Table, etc.

Such recursion could help simplify implementing multitenancy and other types of features. But the price is high:

  • The recursion is a bit counter intuitive. A TableField isn't "composed" of a Table and a Field. Much rather, the Table is just a property of the Field. When a Field is expected (e.g. x = 1), then it it counter-intuitive to traverse into a Table
  • Since most Field expressions are probably TableField expressions, the overhead of recursion is quite significant, even when that is hardly ever needed
  • Consistency dictates that we should treat all Named objects alike. Currently, only SchemaImpl and TableImpl traverse into their parent objects (TableImpl still implements UNotYetImplemented, so this isn't strictly a documented feature)

Implementing this means #15020 will get rejected.

@lukaseder
Copy link
Member Author

There are no regressions in unit or integration tests after this change. It appears that I haven't ever worked with the design of the status quo, yet.

However, obviously, some customers may have already started using the existing design, so this is an incompatible change without a backport.

3.14 New query object model automation moved this from To do to Done May 8, 2023
@lukaseder
Copy link
Member Author

(The change is acceptable because the traversal / replacement feature is still experimental)

@lukaseder lukaseder changed the title Let SchemaImpl and TableImpl implement UEmpty Let SchemaImpl and TableImpl avoid traversal and replacement recursion May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant