Thread: about EncodeDateTime() arguments
We currently have void EncodeDateTime(struct pg_tm * tm, fsec_t fsec, int *tzp, char **tzn, int style, char *str) but tzn isn't used anywhere, only *tzn is used everywhere. Wouldn't it be clearer to remove that one level of indirection and instead have the signature be void EncodeDateTime(struct pg_tm * tm, fsec_t fsec, int *tzp, char *tzn, int style, char *str) or better yet void EncodeDateTime(struct pg_tm * tm, fsec_t fsec, const int *tzp, const char *tzn, int style, char *str)
Peter Eisentraut <peter_e@gmx.net> writes:
> We currently have
> void EncodeDateTime(struct pg_tm * tm, fsec_t fsec, int *tzp, char **tzn, int style, char *str)
> but tzn isn't used anywhere, only *tzn is used everywhere. Wouldn't it
> be clearer to remove that one level of indirection and instead have the
> signature be
> void EncodeDateTime(struct pg_tm * tm, fsec_t fsec, int *tzp, char *tzn, int style, char *str)
> or better yet
> void EncodeDateTime(struct pg_tm * tm, fsec_t fsec, const int *tzp, const char *tzn, int style, char *str)
It appears to me that null-ness of tzp and tzn are used as a 3-way flag
to identify the style of timezone output wanted (none, numeric, or alpha).
It would probably be better yet if it went like
enum tzstyle, int tzp, const char *tzn
where tzp or tzn would be examined only if tzstyle said so.
regards, tom lane
On lör, 2012-03-10 at 18:47 -0500, Tom Lane wrote: > > void EncodeDateTime(struct pg_tm * tm, fsec_t fsec, const int *tzp, const char *tzn, int style, char *str) > > It appears to me that null-ness of tzp and tzn are used as a 3-way flag > to identify the style of timezone output wanted (none, numeric, or alpha). > It would probably be better yet if it went like > > enum tzstyle, int tzp, const char *tzn > > where tzp or tzn would be examined only if tzstyle said so. It's not quite a three-way flag, because it also depends on the style, which time zone style is used. But I liked the idea of making "print the time zone" more explicit and getting rid of the funny pointers. I added a bit of documentation and code deduplication in the attached patch, and it already looks much more understandable.
Attachment
Peter Eisentraut <peter_e@gmx.net> writes:
> On lör, 2012-03-10 at 18:47 -0500, Tom Lane wrote:
>> It appears to me that null-ness of tzp and tzn are used as a 3-way flag
>> to identify the style of timezone output wanted (none, numeric, or alpha).
> It's not quite a three-way flag, because it also depends on the style,
> which time zone style is used. But I liked the idea of making "print
> the time zone" more explicit and getting rid of the funny pointers. I
> added a bit of documentation and code deduplication in the attached
> patch, and it already looks much more understandable.
Yeah, looks nicer to me too.
Should we propagate this fix into ecpg's copy of the code as well?
I'm not sure how far the backend has diverged from that copy.
regards, tom lane