On 2015-07-08 14:11:59 +0900, Michael Paquier wrote: > Arg... I thought I triggered a couple of weeks a problem in this code > path when desc->arg_arraytype[i] is InvalidOid with argtypes == NULL. > Visibly I did something wrong... > > Speaking of which, shouldn't this thing at least use OidIsValid? > - if (fcinfo->flinfo->fn_oid) > + if (OidIsValid(fcinfo->flinfo->fn_oid)) > get_func_signature(fcinfo->flinfo->fn_oid, &argtypes, &nargs);
We do the current type of tests in a bunch of places, I'd not modify code just to change it. But since apparently the whole test is pointless, I could see replacing it by an assert. > >> 4) Return result of timestamp2tm is not checked 2 times in > >> GetCurrentDateTime and GetCurrentTimeUsec, while all the other 40 > >> calls of this function do sanity checks. Returning > >> ERRCODE_DATETIME_VALUE_OUT_OF_RANGE in case of an error would be good > >> for consistency. See 0004. (issue reported previously here > >> CAB7nPqRSk=J8eUdd55fL-w+k=8sDTHLVBt-cgG9jWN=vo2o...@mail.gmail.com) > > > > But the other cases accept either arbitrary input or perform timezone > > conversions. Not the case here, no? > > The callers of GetCurrentDateTime() and GetCurrentTimeUsec() do some > work on pg_tm, see time_timetz() that does accept some input > DecodeDateTime() for example. So what? GetCurrentDateTime()'s returned data is still pretty much guaranteed to be correct unless a lot of things have gone wrong previously? > In any case, we are going to need at least (void) in front of those calls. We're "needing" nothing of the sort. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
