Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Add functional dependency check and aggregate try_new schema #8584

Merged
merged 3 commits into from
Dec 20, 2023
Merged

Bugfix: Add functional dependency check and aggregate try_new schema #8584

merged 3 commits into from
Dec 20, 2023

Conversation

mustafasrepo
Copy link
Contributor

Which issue does this PR close?

Closes #.

Rationale for this change

Make sure schema doesn't change for Aggregate during recreation. Fix functional dependency for wildcard expressions

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added logical-expr Logical plan and expressions core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Dec 19, 2023
@ozankabak ozankabak changed the title Add functional dependency check and aggregate try_new schema Bugfix: Add functional dependency check and aggregate try_new schema Dec 19, 2023
Copy link
Contributor

@ozankabak ozankabak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @mustafasrepo. This is a small bugfix PR resolving issues around the functional dependency mechanism and modification of aggregate executors during planning, but let's keep it open for a day or two in case anyone wants to quickly check it out.

@@ -2162,4 +2194,56 @@ mod tests {
assert_eq!(res, common_requirement);
Ok(())
}

#[test]
fn test_agg_exec_same_schema() -> Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any way that this can be tested from SQL? Or does it require a programatic test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I tried to produce this error from a SQL test. However couldn't reproduce it. The reason may be related to the some rules in physical optimizer has schema_check flag false. But I am not sure, I couldn't come up with a reproducer from the SQL query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants