Peter Smith <smithpb2...@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