alamb commented on code in PR #11896:
URL: https://github.com/apache/datafusion/pull/11896#discussion_r1711636158
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -695,6 +696,23 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
not_impl_err!("Struct not supported by ExprPlanner:
{create_struct_args:?}")
}
+ fn parse_tuple(
+ &self,
+ schema: &DFSchema,
+ planner_context: &mut PlannerContext,
+ values: Vec<SQLExpr>,
+ ) -> Result<Expr> {
+ match values.first() {
+ Some(SQLExpr::Identifier(_)) | Some(SQLExpr::Value(_)) => {
+ self.parse_struct(schema, planner_context, values, vec![])
+ }
+ None => not_impl_err!("Empty tuple not supported yet"),
+ _ => {
Review Comment:
I was trying to figure out how to write a negative test for this case,
however anything I could try seems to work
DataFusion CLI v41.0.0
> create table foo as values (1);
```sql
> select * from foo where (column1) IN ((column1+1), (2));
+---------+
| column1 |
+---------+
+---------+
0 row(s) fetched.
Elapsed 0.008 seconds.
> select * from foo where (column1) IN ((column1+1), (2), (1));
+---------+
| column1 |
+---------+
| 1 |
+---------+
1 row(s) fetched.
Elapsed 0.010 seconds.
```
##########
datafusion/sqllogictest/test_files/struct.slt:
##########
@@ -236,3 +233,44 @@ query ?
select {'animal': {'cat': 1, 'dog': 2, 'bird': {'parrot': 3, 'canary': 1}},
'genre': {'fiction': ['mystery', 'sci-fi', 'fantasy'], 'non-fiction':
{'biography': 5, 'history': 7, 'science': {'physics': 2, 'biology': 3}}},
'vehicle': {'car': {'sedan': 4, 'suv': 2}, 'bicycle': 3, 'boat': ['sailboat',
'motorboat']}, 'weather': {'sunny': True, 'temperature': 25.5, 'wind':
{'speed': 10, 'direction': 'NW'}}};
----
{animal: {cat: 1, dog: 2, bird: {parrot: 3, canary: 1}}, genre: {fiction:
[mystery, sci-fi, fantasy], non-fiction: {biography: 5, history: 7, science:
{physics: 2, biology: 3}}}, vehicle: {car: {sedan: 4, suv: 2}, bicycle: 3,
boat: [sailboat, motorboat]}, weather: {sunny: true, temperature: 25.5, wind:
{speed: 10, direction: NW}}}
+
+# test tuple as struct
+query B
+select ('x', 'y') = ('x', 'y');
+----
+true
+
+query B
+select ('x', 'y') = ('y', 'x');
+----
+false
+
+query error DataFusion error: Error during planning: Cannot infer common
argument type for comparison operation Struct.*
+select ('x', 'y') = ('x', 'y', 'z');
+
+query B
+select ('x', 'y') IN (('x', 'y'));
+----
+true
+
+query B
+select ('x', 'y') IN (('x', 'y'), ('y', 'x'));
+----
+true
+
+query I
+select a from values where (a, c) = (1, 'a');
Review Comment:
I recommend we also do a negative test, like
```
select a from values where (a, c) = (100, 'a');
```
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -780,6 +781,31 @@ fn coerce_numeric_type_to_decimal256(numeric_type:
&DataType) -> Option<DataType
}
}
+fn struct_coercion(lhs_type: &DataType, rhs_type: &DataType) ->
Option<DataType> {
Review Comment:
Could you please document what the intended rules for coercion are, in
English, as a comment? That would help me understand if the code implements the
intent correctly
I think the result is if all the types can be cocerced, then the output is a
struct with fields named `c1` , `c2`, etc
I wonder if it would be better / easier to understand if the names of the
fields were preserved (aka the field names from lhs_fields)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
