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

Add test to verify count aggregate function should not be nullable #12085

Closed
wants to merge 2 commits into from

Conversation

HuSen8891
Copy link
Contributor

Which issue does this PR close?

Closes #12077

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added physical-expr Physical Expressions sqllogictest SQL Logic Tests (.slt) labels Aug 20, 2024
@@ -5364,6 +5364,11 @@ SELECT MAX(col0) FROM empty WHERE col0=1;
----
NULL

query I
Copy link
Contributor

Choose a reason for hiding this comment

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

On main this query fails like this:

creatDataFusion CLI v41.0.0
> create table empty;
0 row(s) fetched.
Elapsed 0.008 seconds.

> select distinct count() from empty;
CombinePartialFinalAggregate
caused by
Internal error: PhysicalOptimizer rule 'CombinePartialFinalAggregate' failed, due to generate a different schema, original schema: Schema { fields: [Field { name: "count()", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }], metadata: {} }, new schema: Schema { fields: [Field { name: "count()", data_type: Int64, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }], metadata: {} }.
This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker

@alamb
Copy link
Contributor

alamb commented Aug 20, 2024

Thank you for this contribution @HuSen8891

@HuSen8891
Copy link
Contributor Author

issue #12077 already solved, close this.

@HuSen8891 HuSen8891 closed this Aug 21, 2024
@alamb
Copy link
Contributor

alamb commented Aug 21, 2024

Thanks @HuSen8891 -- I double checked and it does seem to be fixed on main. However, I don't think we have the test.

Could you please update the PR to just have the additional test?

DataFusion CLI v41.0.0
> create table empty;
0 row(s) fetched.
Elapsed 0.007 seconds.

> select distinct count() from empty;
+---------+
| count() |
+---------+
| 0       |
+---------+
1 row(s) fetched.
Elapsed 0.010 seconds.

>

@alamb alamb reopened this Aug 21, 2024
@alamb alamb changed the title Fix: count aggregate function should not be nullable Add test to verify count aggregate function should not be nullable Aug 21, 2024
@HuSen8891 HuSen8891 closed this Aug 21, 2024
@HuSen8891 HuSen8891 deleted the datafusion branch August 21, 2024 16:03
@HuSen8891
Copy link
Contributor Author

HuSen8891 commented Aug 21, 2024

Thanks @HuSen8891 -- I double checked and it does seem to be fixed on main. However, I don't think we have the test.

Could you please update the PR to just have the additional test?

DataFusion CLI v41.0.0
> create table empty;
0 row(s) fetched.
Elapsed 0.007 seconds.

> select distinct count() from empty;
+---------+
| count() |
+---------+
| 0       |
+---------+
1 row(s) fetched.
Elapsed 0.010 seconds.

>

Sure. Current branch is deleted, I'll add the test in another PR.

@alamb
Copy link
Contributor

alamb commented Aug 21, 2024

Sure. Current branch is deleted, I'll add the test in another PR.

PR was #12100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
physical-expr Physical Expressions sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A simple count() query caused Internal Error in PhysicalOptimizer (SQLancer)
2 participants