Implement unparse IS_NULL to String and enhance the tests
#10529
Conversation
alamb
left a comment
There was a problem hiding this comment.
Thank you for the contribution @goldmedal -- this code looks like a great first contribution.
I just kicked off the CI checks and once we get them to pass successfully I'll merge this PR in
| Expr::IsNull(expr) => { | ||
| Ok(ast::Expr::IsNull(Box::new(self.expr_to_sql(expr)?))) | ||
| } | ||
| Expr::IsNotFalse(_) => not_impl_err!("Unsupported Expr conversion: {expr:?}"), |
There was a problem hiding this comment.
Ah it looks like I maybe mis read the code originally and the other missing expression is Expr::IsNotFalse (rather than Expr::IsNotNull)
There was a problem hiding this comment.
Thanks for reviewing! I think I can fix IsNotFalse in another PR. What do you think?
There was a problem hiding this comment.
I think a second PR will be great
yyy1000
left a comment
There was a problem hiding this comment.
LGTM.
You need to run 'cargo fmt' to format the code,
also I think these additional test cases may not be necessary but it's good to keep them.
|
Thanks again @goldmedal -- welcome to the project! |
…0529) * implement unparse is_null and add test * format the code
Which issue does this PR close?
Closes #10519
Rationale for this change
What changes are included in this PR?
I found that
is_not_nullhas been supported. I only implement 'is_null` here.Are these changes tested?
Yes, I also added more test cases for
is_nullandis_not_null(e.g. function call and case when).Are there any user-facing changes?
No