jayzhan211 commented on code in PR #24799:
URL: https://github.com/apache/datafusion/pull/24799#discussion_r4056446672
##########
datafusion/expr/src/logical_plan/builder.rs:
##########
@@ -1894,7 +1894,12 @@ pub fn build_join_schema(
/// Both `ON` and `USING` preserve all qualified input fields. SQL wildcard
/// expansion handles the unqualified `USING` key as a single column.
pub fn build_asof_join_schema(left: &DFSchema, right: &DFSchema) ->
Result<DFSchema> {
- build_join_schema(left, right, &JoinType::Left)
+ // ASOF emits exactly one output row for each left row. Unlike a general
+ // left join, it cannot duplicate left rows, so left dependencies retain
+ // their modes. Right dependencies do not hold because one right row may
+ // match multiple left rows.
+ build_join_schema(left, right, &JoinType::Left)?
+ .with_functional_dependencies(left.functional_dependencies().clone())
Review Comment:
Replacing the FD set with `left.functional_dependencies()` drops the
right-side deps that `JoinType::Left` propagated (`Multi`, null-downgraded).
Those stay valid for ASOF — "one right row may match many left rows" only rules
out `Single`, not `Multi`. This regresses a query that plans on `main`.
Repro (passes on `main`, fails here with `Column in SELECT must be in GROUP
BY or an aggregate function … "r.v"`):
```sql
CREATE TABLE fl(id INT PRIMARY KEY, grp TEXT, ts INT) AS VALUES
(1, 'A', 1), (2, 'A', 4), (3, 'A', 7), (4, 'B', 2), (5, 'C', 1);
CREATE TABLE fr(id INT PRIMARY KEY, grp TEXT, ts INT, v INT) AS VALUES
(10, 'A', 0, 100), (11, 'A', 5, 110), (12, 'B', 1, 120);
SELECT r.id, r.v
FROM fl l ASOF JOIN fr r MATCH_CONDITION (l.ts >= r.ts) ON l.grp = r.grp
GROUP BY r.id;
```
Fix (verified locally: repro + `asof_join.slt` pass; needs
`FunctionalDependencies` imported, and the unit test should assert left modes
kept + right deps present as `Multi`/nullable):
```diff
- build_join_schema(left, right, &JoinType::Left)?
-
.with_functional_dependencies(left.functional_dependencies().clone())
+ let schema = build_join_schema(left, right, &JoinType::Left)?;
+ // `JoinType::Left` yields `[left as Multi.., right as Multi +
nullable..]`.
+ // Keep the right part and restore the left modes.
+ let n_left = left.functional_dependencies().len();
+ let right_deps = schema.functional_dependencies()[n_left..].to_vec();
+ let mut deps = left.functional_dependencies().clone();
+ // A key that is unique in the output determines every output column.
+ deps.extend_target_indices(schema.fields().len());
+ deps.extend(FunctionalDependencies::new(right_deps));
+ schema.with_functional_dependencies(deps)
```
Please add the repro above to `asof_join.slt`. The `extend_target_indices`
line is optional, but it makes `SELECT l.id, r.v … GROUP BY l.id` plan.
--
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]
