Peter Smith <smithpb2250@gmail.com> writes:
> I only have a couple of questions, more for curiosity than anything else.
> 1. Why is there sometimes an extra *tm = &tt; variable introduced?
> (e.g. GetSQLCurrentTime, GetSQLLocalTime). Why not just declare struct
> pg_tm tm; and pass the &tm same as what GetSQLCurrentDate does?
That's lost in the mists of time, although one could guess that the
original author preferred to write "tm->somefield" uniformly both
in functions that originate a struct pg_tm and those that receive
a pointer to it. But nobody has adopted that idea elsewhere in PG,
so it seems like a confusing anachronism to me.
In this particular patch, I got rid of the extra variable in
GetSQLCurrentDate because I was rewriting it pretty completely
anyway, but desisted from doing so in functions that only needed
minor tweaks. YMMV.
> 2. Shouldn't the comment "/* This is just a convenience wrapper for
> GetCurrentTimeUsec */" be in the function comment for
> GetCurrentDateTime, instead of in the function body?
Done.
> Is there anything else I should be doing to help get this committed?
> IIUC it seems ready as-is.
I think so too, so I pushed it.
regards, tom lane