delta-io / delta

An open-source storage framework that enables building a Lakehouse architecture with compute engines including Spark, PrestoDB, Flink, Trino, and Hive and APIs
https://delta.io
Apache License 2.0
7.18k stars 1.62k forks source link

[Kernel] Change comparator expression to lazy evaluation #2853

Closed zzl-7 closed 2 weeks ago

zzl-7 commented 2 months ago

Which Delta project/connector is this regarding?

Description

Resolves https://github.com/delta-io/delta/issues/2541

How was this patch tested?

Unit test in delta/kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala

Does this PR introduce any user-facing changes?

zzl-7 commented 2 months ago

If code reviewer is ok with the approach, I can patch the PR with cleanup of old code and add more documentation Thank you

zzl-7 commented 2 months ago

@vkorukanti thanks for code reviewing this PR, I have addressed everything, can you take a look at it again? Thank you

zzl-7 commented 1 month ago

Hi @vkorukanti , is there any additional change that you would like me to do? If not can we merge the code? Thanks

vkorukanti commented 1 month ago

@zzl-7, sorry this is getting delayed. Will take a look at this week for sure.

zzl-7 commented 1 month ago

@zzl-7, sorry this is getting delayed. Will take a look at this week for sure.

@vkorukanti , No worries, please let me know if there is anything I can do on my side to help :) Thanks

zzl-7 commented 1 month ago

Hi @vkorukanti , just want to follow up on if there any additional change you would like me to do Thanks :)

zzl-7 commented 2 weeks ago

@vkorukanti Thanks for the code review, I have fixed all suggestions, can you take a look at it again when you are free? Thank you

zzl-7 commented 2 weeks ago

@zzl-7 Just one last comment. The VectorComparator is no longer used. Can you remove it? Let me know, I can also do it before merging it.

(forgot to comment while waiting for github action) good catch, just removed it , thanks :)