[Alvaro Herrera <alvherre@alvh.no-ip.org>]

> diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
> index 233b7b1cc9..df766cdec1 100644
> --- a/src/backend/parser/parse_expr.c
> +++ b/src/backend/parser/parse_expr.c
> @@ -3583,6 +3583,7 @@ coerceJsonFuncExpr(ParseState *pstate, Node *expr,
>  	Node	   *res;
>  	int			location;
>  	Oid			exprtype = exprType(expr);
> +	int32		baseTypmod = returning->typmod;
>  
>  	/* if output type is not specified or equals to function type, return */
>  	if (!OidIsValid(returning->typid) || returning->typid == exprtype)
> @@ -3611,10 +3612,19 @@ coerceJsonFuncExpr(ParseState *pstate, Node *expr,
>  		return (Node *) fexpr;
>  	}
>  
> +	/*
> +	 * For domains, consider the base type's typmod to decide whether to setup
> +	 * an implicit or explicit cast.
> +	 */
> +	if (get_typtype(returning->typid) == TYPTYPE_DOMAIN)
> +		(void) getBaseTypeAndTypmod(returning->typid, &baseTypmod);

I didn't review this patch in detail, but I just noticed this tiny bit
and wanted to say that I don't like this coding style where you
initialize a variable to a certain value, and much later you override it
with a completely different value.  It seems much clearer to leave it
uninitialized at first, and have both places that determine the value
together,

    if (get_typtype(returning->typid) == TYPTYPE_DOMAIN)
        (void) getBaseTypeAndTypmod(returning->typid, &baseTypmod);
    else
        baseTypmod = returning->typmod;

Not only because in the domain case the initializer value is a downright
lie, but also because of considerations such as if you later add code
that uses the variable in between those two places, you'd be introducing
a bug in the domain case because it hasn't been set.  With the coding I
propose, the compiler immediately tells you that the initialization is
missing.


TBH I'm not super clear on why we decide on explicit or implicit cast
based on presence of a typmod.  Why isn't it better to always use an
implicit one?

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Small aircraft do not crash frequently ... usually only once!"
                                  (ponder, http://thedailywtf.com/)