add support for json expression methods

[meeshal]

This PR adds support for json expression methods mentioned in #4216

• is_json_object
• is_not_json_object
• is_json_array
• is_not_json_array
• is_json_scalar
• is_not_json_scalar

[guissalustiano]

Thanks for opening the PR!
I was playing with these operators on the shell and noticed that the IS JSON operator does not support int, boolean, or array. I could only make it work with text type.
If these operands are only supported by text type, I think we should restrict these methods to only diesel text type.

I also notice that SELECT NULL IS JSON SCALLAR returns NULL, so I would like some test showing that this case does not break anything.

[meeshal]

@guissalustiano , I moved json expression methods under PgTextExpressionMethods as you suggested. When it comes to SELECT NULL ... I think that is how psql works and null will always will be returned, it applies to other methods as well. I added a test in is_json_scalar.

[guissalustiano]

Thanks!

[weiznich]

I'm sorry to took so long to add this here, but the nullability handling is still not correct. I've added a more detailed comment in the relevant location explaining what needs to change there. If you need more help to fix this, just let me know. I can also try to change the macro myself, but that won't happen before next week.

[weiznich]

I'm sorry to bring this up now, but this is an issue. In this case is_json_scalar() should evaluate to Nullable<Bool> instead. It's actually a short coming of the postfix_operator!() macro that I forgot about. The diesel::infix_operator!() already handles this internally:

diesel/diesel/src/expression/operators.rs

Lines 296 to 330 in 2090344

	name = $name:ident,
	operator = $operator:expr,
	return_ty = NullableBasedOnArgs ($($return_ty:tt)+),
	backend_ty_params = $backend_ty_params:tt,
	backend_ty = $backend_ty:ty,
	) => {
	$crate::__diesel_infix_operator!(
	name = $name,
	operator = $operator,
	return_ty = (
	$crate::sql_types::is_nullable::MaybeNullable<
	$crate::sql_types::is_nullable::IsOneNullable<
	<T as $crate::expression::Expression>::SqlType,
	<U as $crate::expression::Expression>::SqlType
	>,
	$($return_ty)+
	>
	),
	expression_bounds = (
	$crate::sql_types::is_nullable::IsSqlTypeNullable<
	<T as $crate::expression::Expression>::SqlType
	>: $crate::sql_types::OneIsNullable<
	$crate::sql_types::is_nullable::IsSqlTypeNullable<
	<U as $crate::expression::Expression>::SqlType
	>
	>,
	$crate::sql_types::is_nullable::IsOneNullable<
	<T as $crate::expression::Expression>::SqlType,
	<U as $crate::expression::Expression>::SqlType
	>: $crate::sql_types::MaybeNullableType<$($return_ty)+>,
	),
	backend_ty_params = $backend_ty_params,
	backend_ty = $backend_ty,
	);
	};

Something like this is also needed for the postfix_operator!() macro, although:

• We need to make sure that's not the new default, as that would be a breaking chaneg
• It's likely simpler as we just have one argument instead of two.

We likely just need to introduce an explict opt-in branch to this macro that sets the return type to

$crate::sql_types::is_nullable::MaybeNullable<<T as Expression>::SqlType>

after that this test should just compile as

diesel::select(sql::<Nullable<Text>>("NULL").is_json_scalar()).get_result::<Option<bool>>(conn)?;

[meeshal]

I am not an expert when it comes to macros but I am thinking about sth like postfix_operator!(IsJsonScalar, " IS JSON SCALAR", ConditionalNullability Bool, backend: Pg); to make it backward compatible. Implementations without ConditionalNullability Bool specified will evaluate to the legacy branch. @weiznich , what do you think ?

[weiznich]

That would be a fine solution 👍

[meeshal]

@weiznich , I added ConditionalNullability support to postfix_operator. Please let me know if it makes sense.

[weiznich]

Looks good now, thanks for fixing 👍

(I made a minor adjustment to simplify the calculated return type a bit)