add cast support inside values

[kmitchener]

Which issue does this PR close?

Closes #3446 .

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

[codecov-commenter]

Codecov Report

Merging #3447 (b7ed1c0) into master (8df5496) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3447      +/-   ##
==========================================
- Coverage   85.69%   85.69%   -0.01%     
==========================================
  Files         298      298              
  Lines       54644    54658      +14     
==========================================
+ Hits        46826    46837      +11     
- Misses       7818     7821       +3     

Impacted Files	Coverage Δ
datafusion/core/tests/sql/select.rs	99.78% <100.00%> (+<0.01%)	⬆️
datafusion/sql/src/planner.rs	80.94% <100.00%> (+0.05%)	⬆️
datafusion/expr/src/window_frame.rs	92.43% <0.00%> (-0.85%)	⬇️
datafusion/core/src/physical_plan/metrics/value.rs	86.93% <0.00%> (-0.51%)	⬇️
datafusion/expr/src/logical_plan/plan.rs	77.19% <0.00%> (-0.17%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[alamb]

Looks good @kmitchener 👍 thank you

Looking at this code, I wonder why there are special cases for Expr values at all in the sql planner. It seems like this is a special case of expression coercion (aka we are trying to coerce / cast all arguments to the same type).

[kmitchener]

Looks good @kmitchener 👍 thank you

Looking at this code, I wonder why there are special cases for Expr values at all in the sql planner. It seems like this is a special case of expression coercion (aka we are trying to coerce / cast all arguments to the same type).

Hmm, do you mean that maybe we can set some rules about which exprs are valid and use those to filter the input to values() rather than explicitly listing which expressions are valid? It does seem like there should be a better way to do this, because values() can take most (any?) expression that's not aggregate, like substring or case or even try_cast for that matter .. maybe a follow-on issue for that? that's beyond me at the moment.

[ursabot]

Benchmark runs are scheduled for baseline = c5c1dae and contender = 3e56a0f. 3e56a0f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

[alamb]

Hmm, do you mean that maybe we can set some rules about which exprs are valid and use those to filter the input to values() rather than explicitly listing which expressions are valid? It does seem like there should be a better way to do this, because values() can take most (any?) expression that's not aggregate, like substring or case or even try_cast for that matter ..

Yes, exactly this.

maybe a follow-on issue for that? that's beyond me at the moment.

I will do so