fix NULL <op> column evaluation, tests for same

[alamb]

Which issue does this PR close?

re #1179 and #2482 .

Rationale for this change

While adding tests suggested in #2481 for reversed arguments, it uncovered a bug if the argument order was different.

Previously we tested column1 < NULL but not NULL < column1. When I did so I got

---- sql::expr::comparisons_with_null_neq_reverse stdout ----
thread 'sql::expr::comparisons_with_null_neq_reverse' panicked at 'called `Result::unwrap()` on an `Err` value: "ArrowError(ExternalError(Internal(\"Cannot convert Int64(NULL) to i64\"))) at Executing physical plan for 'select NULL != column1 from (VALUES (1, 'foo' ,2.3), (2, 'bar', 5.4)) as t': ProjectionExec { expr: [(BinaryExpr { left: TryCastExpr { expr: Literal { value: NULL }, cast_type: Int64 }, op: NotEq, right: Column { name: \"column1\", index: 0 } }, \"NULL NotEq t.column1\")], schema: Schema { fields: [Field { name: \"NULL NotEq t.column1\", data_type: Boolean, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None }], metadata: {} }, input: ProjectionExec { expr: [(Column { name: \"column1\", index: 0 }, \"column1\")], schema: Schema { fields: [Field { name: \"column1\", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None }], metadata: {} }, input: ValuesExec { schema: Schema { fields: [Field { name: \"column1\", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None }, Field { name: \"column2\", data_type: Utf8, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None }, Field { name: \"column3\", data_type: Float64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None }], metadata: {} }, data: [RecordBatch { schema: Schema { fields: [Field { name: \"column1\", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None }, Field { name: \"column2\", data_type: Utf8, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None }, Field { name: \"column3\", data_type: Float64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None }], metadata: {} }, columns: [PrimitiveArray<Int64>\n[\n  1,\n  2,\n], StringArray\n[\n  \"foo\",\n  \"bar\",\n], PrimitiveArray<Float64>\n[\n  2.3,\n  5.4,\n]], row_count: 2 }] }, metrics: ExecutionPlanMetricsSet { inner: Mutex { data: MetricsSet { metrics: [] } } } }, metrics: ExecutionPlanMetricsSet { inner: Mutex { data: MetricsSet { metrics: [] } } } }"', datafusion/core/tests/sql/mod.rs:563:10

What changes are included in this PR?

This PR adds a test exercising the order of expressions with null constants, and fixes evaluation of same

Are there any user-facing changes?

Fewer internal errors

cc @WinkerDu and @yjshen

[alamb]

this is the only surprising change I thought

[alamb]

I wanted to test a bunch of different combinations, so I rewrote the test so I could check them all

[alamb]

Tests below here are new. The tests above were pre-existing

[alamb]

changes here were to make the code more readable (so cargo fmt put it on the same line)

[alamb]

This is the actual fix -- to use binary_array_op_dyn_scalar which @WinkerDu updated to handle null constants rather than binary_array_op_scalar which does not.

I plan to look into removing binary_array_op_scalar completely as a follow on PR.

[WinkerDu]

Sorry for late response, love this fix ❤️

[yjshen]

Sorry for the late review, LGTM!

[alamb]

Thanks @yjshen