Skip to content

remove unalias TableScan filters when create Physical Filter - #8404

Merged
jackwener merged 1 commit into
apache:mainfrom
jackwener:refactor_code
Dec 3, 2023
Merged

remove unalias TableScan filters when create Physical Filter#8404
jackwener merged 1 commit into
apache:mainfrom
jackwener:refactor_code

Conversation

@jackwener

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #.

Rationale for this change

  • remove unalias TableScan filters
  • refactor CreateExternalTable
  • fix typo

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actionsgithub-actionsBot added physical-expr Changes to the physical-expr crates core Core DataFusion crate labels Dec 3, 2023
// doesn't know (nor should care) how the relation was
// referred to in the query
let filters = unnormalize_cols(filters.iter().cloned());
let unaliased: Vec<Expr> = filters.into_iter().map(unalias).collect();

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It's useless

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we have to rely on our test coverage here, and given all the existing tests pass I agree!

@jackwenerjackwenerDec 3, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, existing tests already cover it.
BTW, theoretically and in terms of code, the alias in the filter expression does not affect the schema or output.

@jackwenerjackwener changed the title remove unalias TableScan filtersremove unalias TableScan filters in create PhysicalPlanDec 3, 2023
@jackwenerjackwener changed the title remove unalias TableScan filters in create PhysicalPlanremove unalias TableScan filters when create Physical FilterDec 3, 2023

@alambalamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you @jackwener -- this is a very nice PR 👏

// doesn't know (nor should care) how the relation was
// referred to in the query
let filters = unnormalize_cols(filters.iter().cloned());
let unaliased: Vec<Expr> = filters.into_iter().map(unalias).collect();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we have to rely on our test coverage here, and given all the existing tests pass I agree!

@jackwener
jackwener merged commit 26196e6 into apache:mainDec 3, 2023
@jackwener
jackwener deleted the refactor_code branch December 3, 2023 13:44
appletreeisyellow pushed a commit to appletreeisyellow/datafusion that referenced this pull request Dec 14, 2023
…che#8404)
- remove `unalias` TableScan filters
- refactor CreateExternalTable
- fix typo
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coreCore DataFusion cratephysical-exprChanges to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jackwener@alamb