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

Make our SQL tree fully immutable #32927

Open
Tracked by #34524
roji opened this issue Jan 26, 2024 · 0 comments
Open
Tracked by #34524

Make our SQL tree fully immutable #32927

roji opened this issue Jan 26, 2024 · 0 comments

Comments

@roji
Copy link
Member

roji commented Jan 26, 2024

#32812 and #32815 made significant steps towards making our SQL tree immutable, by removing TableReferenceExpression (which was mutable) and by making TableExpressionBase.Alias immutable.

However, SelectExpression itself still has a mutable (being translated) and immutable (finalized, pushed down) states. This requires us to have separate code paths for each state in various places (e.g. SelectExpression.VisitChildren), slows down translation (since we need to duplicate/clone in various places), and is generally unsafe and not the standard way to work with expressions.

Once this is done, expression cloning can be removed. We may still choose to retain it to have unique table aliases when a fragment is cloned; however, that's mostly an esthetic SQL concern, and the direction there is to stop reusing the same fragment in multiple places in the tree in any case (for better SQL perf, #32277) - at that point the point is moot anyway.

Another reason to do this is e.g. more efficient expression comparison/hash code management: immutability would allow us to cache the hash code, and use it as a first test when doing recursive expression comparison.

Note to self: remember that TpcTablesExpression currently also has a tiny bit of immutability - remove that as part of this as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants