about EncodeDateTime() arguments

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.

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