Skip to content

Conversation

@jackwener
Copy link
Member

@jackwener jackwener commented Jun 14, 2023

Which issue does this PR close?

related #6447

Rationale for this change

related PR #6010

What changes are included in this PR?

This test use wrong select(), so bug is hidden.
correct this test and ignore this PR temporarily

error log

Error: SchemaError(FieldNotFound { field: Column { relation: None, name: "COUNT(*)" }, valid_fields: [Column { relation: None, name: "COUNT(UInt8(1))" }, Column { relation: Some(Bare { table: "t2" }), name: "a" }, Column { relation: Some(Bare { table: "t2" }), name: "b" }, Column { relation: Some(Bare { table: "t2" }), name: "c" }] })

update:

use count(Wildcard).to_string() to resolve problem.

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the core Core DataFusion crate label Jun 14, 2023
@jackwener jackwener force-pushed the ignore_wrong_test branch from 88c6af9 to 14a0350 Compare June 14, 2023 15:37
@jackwener
Copy link
Member Author

This bug is a little difficult, I am trying to repair.

I create PR to fix the problematic tests in advance now.

.filter(out_ref_col(DataType::UInt32, "t1.a").eq(col("t2.a")))?
.aggregate(vec![], vec![count(lit(COUNT_STAR_EXPANSION))])?
.select(vec![count(lit(COUNT_STAR_EXPANSION))])?
.select(vec![col("COUNT(*)")])?
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a dataframe test, maybe could use the expr name directly rather than the hard coded string 🤔

Something like

               let count_star = count(lit(COUNT_STAR_EXPANSION));
                ctx.table("t2")
                    .await?
                    .filter(out_ref_col(DataType::UInt32, "t1.a").eq(col("t2.a")))?
                    .aggregate(vec![], vec![count_star.clone()])?
                    .select(vec![col(count_star.to_string())])?

Copy link
Member Author

Choose a reason for hiding this comment

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

A great idea!

@jackwener jackwener force-pushed the ignore_wrong_test branch from 14a0350 to 6cb2187 Compare June 15, 2023 03:04
@jackwener jackwener changed the title fix: correct wrong test and ignore it. fix: correct wrong test Jun 15, 2023
@jackwener jackwener merged commit c4928b0 into apache:main Jun 15, 2023
@jackwener jackwener deleted the ignore_wrong_test branch June 15, 2023 04:41
.filter(out_ref_col(DataType::UInt32, "t1.a").eq(col("t2.a")))?
.aggregate(vec![], vec![count(lit(COUNT_STAR_EXPANSION))])?
.select(vec![count(lit(COUNT_STAR_EXPANSION))])?
.aggregate(vec![], vec![count(Wildcard)])?
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants