Skip to content

refactor: use get_result_type to replace binary_operator_data_type.#6241

Merged
alamb merged 1 commit intoapache:mainfrom
jackwener:simplify
May 5, 2023
Merged

refactor: use get_result_type to replace binary_operator_data_type.#6241
alamb merged 1 commit intoapache:mainfrom
jackwener:simplify

Conversation

@jackwener
Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #.

Rationale for this change

following #6221

What changes are included in this PR?

  • use get_result_type to replace binary_operator_data_type
  • fix case in get_result_type
  • don't use coerce_type in get_result_type

Are these changes tested?

Are there any user-facing changes?

N/A

@github-actions github-actions bot added core Core DataFusion crate logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels May 5, 2023
@jackwener jackwener requested a review from alamb May 5, 2023 04:43
Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @jackwener -- this looks like yet another significant simplification to me

| Operator::Gt
| Operator::GtEq
| Operator::LtEq
| Operator::RegexMatch
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.

this is a good change

@alamb
Copy link
Copy Markdown
Contributor

alamb commented May 5, 2023

Screenshot 2023-05-05 at 8 00 03 AM

❤️

@alamb alamb merged commit d3c76c4 into apache:main May 5, 2023
@jackwener jackwener deleted the simplify branch May 8, 2023 04:44
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 physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants