> 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/)