Fix between in select query

[capkurmagati]

Which issue does this PR close?

Closes #1196.

1. Added match arms for Expr::Between, Expr::Sort, and Expr::Wildcard and removed default match in expr.rs.
2. Added match arm for Expr::Between in planner.rs.

echo "1" > /tmp/foo.csv
cargo run -p datafusion-cli

CREATE EXTERNAL TABLE foo(x int)
STORED AS CSV
LOCATION '/tmp/foo.csv';

>  select 1 between 5 and 10;
+----------------+
| Boolean(false) |
+----------------+
| false          |
+----------------+
1 row in set. Query took 0.003 seconds.

> select 1 between 5 and 10 from foo;
+----------------+
| Boolean(false) |
+----------------+
| false          |
+----------------+
1 row in set. Query took 0.011 seconds.

> select x between 5 and 10 from foo;
+--------------------------------------+
| foo.x BETWEEN Int64(5) AND Int64(10) |
+--------------------------------------+
| false                                |
+--------------------------------------+
1 row in set. Query took 0.012 seconds.

> explain select x between 5 and 10 from foo;
+---------------+---------------------------------------------------------------------------------------------------------------------+
| plan_type     | plan                                                                                                                |
+---------------+---------------------------------------------------------------------------------------------------------------------+
| logical_plan  | Projection: #foo.x BETWEEN Int64(5) AND Int64(10)                                                                   |
|               |   TableScan: foo projection=Some([0])                                                                               |
| physical_plan | ProjectionExec: expr=[CAST(x@0 AS Int64) >= 5 AND CAST(x@0 AS Int64) <= 10 as foo.x BETWEEN Int64(5) AND Int64(10)] |
|               |   RepartitionExec: partitioning=RoundRobinBatch(8)                                                                  |
|               |     CsvExec: files=[/tmp/foo.csv], has_header=false, batch_size=8192, limit=None                                    |
|               |                                                                                                                     |
+---------------+---------------------------------------------------------------------------------------------------------------------+
2 rows in set. Query took 0.005 seconds.

[jimexist]

thanks!

could you also add an integration test?

[capkurmagati]

@jimexist Sure. Since I'm new to the project, let me drop some newbie questions.

1. planner.rs seems have integration tests using arrow-datafusion/testing. Is that the test you want to add?
2. I couldn't find tests for create_name(e: &Expr, input_schema: &DFSchema) -> Result<String> . Do you also want to add some tests for that?

[xudong963]

• planner.rs seems have integration tests using arrow-datafusion/testing. Is that the test you want to add?
• I couldn't find tests for create_name(e: &Expr, input_schema: &DFSchema) -> Result<String> . Do you also want to add some tests for that?

I think you can add unit tests firstly in datafusion/tests/sql.rs where has tests about other exprs.

[alamb]

I think you can add unit tests firstly in datafusion/tests/sql.rs where has tests about other exprs.

I agree with @xudong963 that a test in sql.rs would be sufficient for this PR. Thank you for the contribution @capkurmagati

[capkurmagati]

@xudong963 @alamb Thanks for the advice. I added some tests for the expr. PTAL.
I will rebase the branch after reflecting your reviews.

[xudong963]

The remaining LGTM, wait for @alamb to review. Nice work @capkurmagati

[alamb]

Thank you so much @capkurmagati -- the tests look great.

[alamb]

There appears to be a conflict in sql.rs. I have merged up from main to resolve it

[alamb]

The test was failing due to the executor having 2 cores, but the output reflected 8 cores -- updated in 8eff3de

[capkurmagati]

@xudong963 Removed the default match arm and added match arms for Expr::Sort, and Expr::Wildcard in create_physical_name just like create_name. PTAL.
@alamb Thanks so much for resolving code conflict and fixing the test. Much appreciated.

[xudong963]

You can directly use Expr::Sort{ .. }

[capkurmagati]

Thanks for the advice. Fixed.

[xudong963]

ditto

[xudong963]

LGTM, thanks @capkurmagati. Let @alamb do the final review and merging.

[alamb]

This is looking great -- thank you @capkurmagati -- I'll plan to merge this once the CI checks pass

[alamb]

Thanks again @capkurmagati !