clflushopt commented on code in PR #23488: URL: https://github.com/apache/datafusion/pull/23488#discussion_r3589218144
##########
datafusion/substrait/src/logical_plan/producer/expr/field_reference.rs:
##########
@@ -76,20 +85,37 @@ pub(crate) fn try_to_substrait_field_reference(
}
}
-/// Convert an outer reference column to a Substrait field reference.
-/// Outer reference columns reference columns from an outer query scope in
correlated subqueries.
-/// We convert them the same way as regular columns since the subquery plan
will be
-/// reconstructed with the proper schema context during consumption.
+/// Convert an outer reference column to a Substrait field reference with an
+/// `OuterReference` root type.
+///
+/// Outer reference columns reference columns from an enclosing query scope in
+/// correlated subqueries. The column is resolved against the producer's stack
+/// of outer schemas (pushed at each subquery boundary), innermost first, and
+/// the resulting `steps_out` records how many query boundaries the reference
+/// crosses (`steps_out = 1` is the immediately enclosing query).
pub fn from_outer_reference_column(
+ producer: &mut impl SubstraitProducer,
+ _field: &FieldRef,
col: &Column,
- schema: &DFSchemaRef,
+ _schema: &DFSchemaRef,
) -> datafusion::common::Result<Expression> {
- // OuterReferenceColumn is converted similarly to a regular column
reference.
- // The schema provided should be the schema context in which the outer
reference
- // column appears. During Substrait round-trip, the consumer will
reconstruct
- // the outer reference based on the subquery context.
- let index = schema.index_of_column(col)?;
- substrait_field_ref(index)
+ let mut steps_out = 1;
+ while let Some(outer_schema) = producer.get_outer_schema(steps_out) {
+ if let Some(index) = outer_schema.maybe_index_of_column(col) {
Review Comment:
I don't think so I believe it bounds to the nearest enclosing scope, which I
confirmed by adding a test for this. To ease review, below is what the test
reproduces when printing out the outputs of each step.
```
SELECT b FROM data WHERE EXISTS (SELECT 1 FROM data WHERE EXISTS (SELECT 1
FROM book WHERE book.isbn = data.a))
```
* Datafusion's plan (unoptimized):
```
Projection: data.b
Filter: EXISTS (<subquery>)
Subquery:
Projection: Int64(1)
Filter: EXISTS (<subquery>)
Subquery:
Projection: Int64(1)
Filter: book.isbn = outer_ref(data.a)
TableScan: book
TableScan: data
TableScan: data
```
* Substrait's plan :
```
{
"version": {
"minorNumber": 85,
"producer": "datafusion"
},
"extensions": [
{
"extensionFunction": {
"extensionUrnReference": 4294967295,
"name": "equal"
}
}
],
"relations": [
{
"root": {
"input": {
"project": {
"common": {
"emit": {
"outputMapping": [
6
]
}
},
"input": {
"filter": {
"input": {
"read": {
"baseSchema": {
"names": [
"a",
"b",
"c",
"d",
"e",
"f"
],
"struct": {
"types": [
{
"i64": {
"nullability": "NULLABILITY_NULLABLE"
}
},
{
"decimal": {
"scale": 2,
"precision": 5,
"nullability": "NULLABILITY_NULLABLE"
}
},
{
"date": {
"nullability": "NULLABILITY_NULLABLE"
}
},
{
"bool": {
"nullability": "NULLABILITY_NULLABLE"
}
},
{
"i32": {
"typeVariationReference": 1,
"nullability": "NULLABILITY_NULLABLE"
}
},
{
"string": {
"nullability": "NULLABILITY_NULLABLE"
}
}
],
"nullability": "NULLABILITY_REQUIRED"
}
},
"namedTable": {
"names": [
"data"
]
}
}
},
"condition": {
"subquery": {
"setPredicate": {
"predicateOp": "PREDICATE_OP_EXISTS",
"tuples": {
"project": {
"common": {
"emit": {
"outputMapping": [
6
]
}
},
"input": {
"filter": {
"input": {
"read": {
"baseSchema": {
"names": [
"a",
"b",
"c",
"d",
"e",
"f"
],
"struct": {
"types": [
{
"i64": {
"nullability":
"NULLABILITY_NULLABLE"
}
},
{
"decimal": {
"scale": 2,
"precision": 5,
"nullability":
"NULLABILITY_NULLABLE"
}
},
{
"date": {
"nullability":
"NULLABILITY_NULLABLE"
}
},
{
"bool": {
"nullability":
"NULLABILITY_NULLABLE"
}
},
{
"i32": {
"typeVariationReference": 1,
"nullability":
"NULLABILITY_NULLABLE"
}
},
{
"string": {
"nullability":
"NULLABILITY_NULLABLE"
}
}
],
"nullability": "NULLABILITY_REQUIRED"
}
},
"namedTable": {
"names": [
"data"
]
}
}
},
"condition": {
"subquery": {
"setPredicate": {
"predicateOp": "PREDICATE_OP_EXISTS",
"tuples": {
"project": {
"common": {
"emit": {
"outputMapping": [
3
]
}
},
"input": {
"filter": {
"input": {
"read": {
"baseSchema": {
"names": [
"isbn",
"title",
"genre"
],
"struct": {
"types": [
{
"i64": {
"nullability":
"NULLABILITY_REQUIRED"
}
},
{
"string": {
"nullability":
"NULLABILITY_NULLABLE"
}
},
{
"string": {
"nullability":
"NULLABILITY_NULLABLE"
}
}
],
"nullability":
"NULLABILITY_REQUIRED"
}
},
"namedTable": {
"names": [
"book"
]
}
}
},
"condition": {
"scalarFunction": {
"arguments": [
{
"value": {
"selection": {
"directReference": {
"structField": {}
},
"rootReference": {}
}
}
},
{
"value": {
"selection": {
"directReference": {
"structField": {}
},
"outerReference": {
"stepsOut": 1
}
}
}
}
],
"outputType": {
"bool": {
"nullability":
"NULLABILITY_NULLABLE"
}
}
}
}
}
},
"expressions": [
{
"literal": {
"i64": "1"
}
}
]
}
}
}
}
}
}
},
"expressions": [
{
"literal": {
"i64": "1"
}
}
]
}
}
}
}
}
}
},
"expressions": [
{
"selection": {
"directReference": {
"structField": {
"field": 1
}
},
"rootReference": {}
}
}
]
}
},
"names": [
"b"
]
}
}
]
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
