Simplify regexp replace handling#32053
Conversation
|
Unfortunately, there was a reason for this complication: #27300 (comment) |
antiguru
force-pushed
the
regexpr_replace_lit_error
branch
from
c33a512 to
7007eaf
Compare
| Err(err) => { | ||
| let mut exprs = mem::take(exprs); | ||
| let source = exprs.swap_remove(0); | ||
| let scalar_type = e.typ(column_types).scalar_type; | ||
| // We need to return `NULL` on `NULL` input, and error otherwise. | ||
| source | ||
| .call_is_null() | ||
| .if_then_else( | ||
| MirScalarExpr::literal_null(scalar_type.clone()), | ||
| MirScalarExpr::literal(Err(err), scalar_type), | ||
| ) | ||
| } |
There was a problem hiding this comment.
@ggevay I changed the error path to return null on null input, and error otherwise. I think this should match the current behavior!
antiguru
force-pushed
the
regexpr_replace_lit_error
branch
from
7007eaf to
d75ea5b
Compare
| let regex = match func::build_regex(pattern, &flags) { | ||
| Ok(regex) => Ok((regex, limit)), | ||
| Err(err) => Err(err), | ||
| *e = match func::build_regex(pattern, &flags) { |
There was a problem hiding this comment.
Please update the above comment. Or you could just remove that comment, since there is also a comment below in the error case.
| let mut exprs = mem::take(exprs); | ||
| let source = exprs.swap_remove(0); | ||
| let scalar_type = e.typ(column_types).scalar_type; | ||
| // We need to return `NULL` on `NULL` input, and error otherwise. |
There was a problem hiding this comment.
Maybe extend the comment a bit. Something like:
"We'd like the same behaviors between the regex argument being dynamic or a literal. Therefore, we need to return NULL on NULL input, and error otherwise, because this is what the original, dynamic function does."
Signed-off-by: Moritz Hoffmann <mh@materialize.com>
Signed-off-by: Moritz Hoffmann <mh@materialize.com>
Signed-off-by: Moritz Hoffmann <mh@materialize.com>
antiguru
force-pushed
the
regexpr_replace_lit_error
branch
from
d75ea5b to
758ff98
Compare
|
Thanks for the review! |
2 participants
Simplify regexp replace handling by using a literal error
Instead of encapsulating a result, replace the function call by a literal error. The hope is that this is semantically equivalent to the existing implementation.
Part of MaterializeInc/database-issues#9138
Motivation
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.