Adam Curtis

Results 71 comments of Adam Curtis

Thanks for the detailed write up @peter-toth . Though I did mention `HashSet` specifically, my suggestion more generally goes along the lines of using the `Hash` implementation in some way...

> I think there's nothing wrong about having a "display" infrastructure, but it shouldn't be used eagerly. As others pointed out, using a hash or any form of numeric ID...

Benchmark results between the two PRs and main ![image](https://github.com/apache/datafusion/assets/1530244/874d2e4d-e993-4cff-ab81-db1f75a9d4a0)

I will take another look at this and see if I can clean it up a bit more.

Alright I think this is in a good state now. I added a dynamic dispatch API which I think could be useful if we ever add `TreeNodeRewriter` impl for `OptimizerRule`,...

I removed the dynamic dispatch API for now because I think there is a better way to write it, but I can't really do that until we get to a...

> I think the logic is correct, although it takes me sometime to understand the difference between the first pass and other pass, but I did not have a better...

@alamb I will try to update this today or tomorrow. I've been putting this off a bit.

> I ran the planning benchmark with this brach an in summary there is basically no performance change Yes that is what I observed as well. I assume the overhead...

I will update this PR with review suggestions (maybe) this weekend