On 11/15/19 2:14 PM, Pavel Stehule wrote: > Hi > > > > For release 13+, I have given some more thought to what should be > done. > I think the bar for altering the behaviour of a function should be > rather higher than we have in the present case, and the longer the > function has been sanctioned by time the higher the bar should be. > However, I think there is a case to be made for providing a non-strict > jsonb_set type function. To advance th4e discussion, attached is a POC > patch that does that. This can also be done as an extension, meaning > that users of back branches could deploy it immediately. I've tested > this against release 12, but I think it could go probably all the way > back to 9.5. The new function is named jsonb_ set_lax, but I'm open to > bikeshedding. > > > I am sending a review of this patch > > 1. this patch does what was proposed and it is based on discussion. > > 2. there are not any problem with patching or compilation, all regress > tests passed. > > 4. code looks well and it is well commented. > > 5. the patch has enough regress tests > > My notes: > > a) missing documentation > > b) error message is not finalized > > + ereport(ERROR, > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > + errmsg("null jsonb value"))); > > Any other looks well, and this function can be very handy. > >
Thanks for the review. I will add some docco.
What would be a better error message? "null jsonb replacement not permitted"?
Maybe ERRCODE_NULL_VALUE_NOT_ALLOWED, and "NULL is not allowed",
errdetail - a exception due setting "null_value_treatment" => raise_exception
and maybe some errhint - "Maybe you would to use Jsonb NULL - "null"::jsonb"
I don't know, but in this case, the exception should be verbose. This is "rich" function with lot of functionality
cheers
andrew
-- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
