Skip to content

Conversation

@Weijun-H
Copy link
Member

@Weijun-H Weijun-H commented Jan 8, 2024

Which issue does this PR close?

Closes #.

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 the physical-expr Changes to the physical-expr crates label Jan 8, 2024
pub fn array_except(args: &[ArrayRef]) -> Result<ArrayRef> {
if args.len() != 2 {
return internal_err!("array_except needs two arguments");
return exec_err!("array_except needs two arguments");
Copy link
Contributor

@jayzhan211 jayzhan211 Jan 8, 2024

Choose a reason for hiding this comment

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

When to use internal_err, when to use exec_err?

Copy link
Contributor

Choose a reason for hiding this comment

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

internal_err: something that wasn't expected/anticipated by the implementation and that is most likely a bug (the error message even encourages users to open a bug report). I user should not be able to trigger this under normal circumstances. Note that I/O errors (or any error that happens due to external systems) do NOT fall under this category (there's an external error variant for that)

exec_err: a processing error that happens during execution, e.g. the user passed malformed arguments to a SQL method, opened a CSV file that is broken, tried to divide an integer by zero. these errors shouldn't happen for a "good" query + "good" data, but since both the query and the data can easily be broken or misaligned, these are common occurrences in ETL systems / databases.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is such a great explanation I figured I would encode it in the comments of DataFusionError in its own PR: #8792

@alamb
Copy link
Contributor

alamb commented Jan 8, 2024

Thanks @Weijun-H and @crepererum

@Weijun-H Weijun-H mentioned this pull request Jan 31, 2024
Tangruilin added a commit to Tangruilin/arrow-datafusion that referenced this pull request Jan 31, 2024
…n.rs to exec_err!

Signed-off-by: tangruilin <tang.ruilin@foxmail.com>
alamb pushed a commit that referenced this pull request Jan 31, 2024
…o exec_err! (#9083)

Signed-off-by: tangruilin <tang.ruilin@foxmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants