Re: [HACKERS] removing abstime, reltime, tinterval.c, spi/timetravel

Hi,

On 2018-10-12 19:47:40 -0400, Tom Lane wrote:
> BTW, I was annoyed while looking things over that this patch had broken
> a couple of comments in opr_sanity.sql:
>
> @@ -823,7 +823,6 @@ WHERE a.aggfnoid = p.oid AND
>
> -- Cross-check transfn against its entry in pg_proc.
> -- NOTE: use physically_coercible here, not binary_coercible, because
> --- max and min on abstime are implemented using int4larger/int4smaller.
> SELECT a.aggfnoid::oid, p.proname, ptr.oid, ptr.proname
> FROM pg_aggregate AS a, pg_proc AS p, pg_proc AS ptr
> WHERE a.aggfnoid = p.oid AND
> @@ -978,7 +977,6 @@ WHERE a.aggfnoid = p.oid AND
> -- Check that all combine functions have signature
> -- combine(transtype, transtype) returns transtype
> -- NOTE: use physically_coercible here, not binary_coercible, because
> --- max and min on abstime are implemented using int4larger/int4smaller.
>
> SELECT a.aggfnoid, p.proname
> FROM pg_aggregate as a, pg_proc as p
>
> Just removing a fraction of the sentence is not good.

Fair.

> So I went looking for a different example to plug in there, and soon
> found that there weren't any. If you change all the physically_coercible
> calls in that script to binary_coercible, its output doesn't change.

> I'm thinking that we ought to do that, and just get rid of
> physically_coercible(), so that we have a tighter, more semantically
> meaningful set of checks here. We can always undo that if we ever
> have occasion to type-cheat like that again, but offhand I'm not sure
> why we would do so.

Hm, I wonder if it's not a good idea to leave the test there, or rewrite
it slightly, so we have a a more precise warning about cheats like that?

I also don't really see why we'd introduce new hacks like reusing
functions like that, but somebody might do it while hacking something
together and forget about it.

Probably still not worth it?

Greetings,

Andres Freund