refactor: fix error macros hygiene (always import DataFusionError)
#9366
Conversation
github-actions
bot
added
sql
crepererum
force-pushed
the
crepererum/macro_hygiene
branch
from
83c5dfb to
e114c7c
Compare
alamb
changed the title
DataFusionError)
There was a problem hiding this comment.
Thank you @crepererum -- this has bothered me for a long time as well. It is a very nice paper cut improvement
😍
cc @comphead
| fn merge_grouping_set<T: Clone>(left: &[T], right: &[T]) -> Result<Vec<T>> { | ||
| check_grouping_set_size_limit(left.len() + right.len())?; | ||
| Ok(left.iter().chain(right.iter()).cloned().collect()) | ||
| } | ||
|
|
||
| /// Compute the cross product of two grouping_sets | ||
| /// | ||
| /// # Example |
crepererum
force-pushed
the
crepererum/macro_hygiene
branch
from
e114c7c to
8b784f2
Compare
There was a problem hiding this comment.
thanks @crepererum I love it.
Maybe we can invent a better naming for the mod? hygiene is a little bit unclear what is supposed to do
crepererum
force-pushed
the
crepererum/macro_hygiene
branch
from
8b784f2 to
2f3e097
Compare
I'm open to suggestions. To me "macro hygiene" is an official term. |
Ic, good point. would you mind adding the this WIKI link to the macro file? I think it would be useful for ppl like me who face hygiene macro term the first time :) |
crepererum
force-pushed
the
crepererum/macro_hygiene
branch
from
388b36a to
ac63469
Compare
|
Rebased and a addressed all comments. Merging this due to the high conflict potential (we can always file follow-up PRs). |
Which issue does this PR close?
-
Rationale for this change
Error macros should be hygienic, i.e. they should be self-contained and don't require the user to import additional symbols.
What changes are included in this PR?
Are these changes tested?
Regression test.
Are there any user-facing changes?
User no longer needs to import
DataFusionErrorjust to use error macros (e.g.plan_err!).