Thread

1. about EncodeDateTime() arguments

   Peter Eisentraut <peter_e@gmx.net> — 2012-03-10T22:39:25Z

   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)
   
   
   
   
   

2. Re: about EncodeDateTime() arguments

   Tom Lane <tgl@sss.pgh.pa.us> — 2012-03-10T23:47:48Z

   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
   
   

3. Re: about EncodeDateTime() arguments

   Peter Eisentraut <peter_e@gmx.net> — 2012-03-13T19:41:56Z

   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.
   

4. Re: about EncodeDateTime() arguments

   Tom Lane <tgl@sss.pgh.pa.us> — 2012-03-13T21:26:26Z

   Peter Eisentraut <peter_e@gmx.net> writes:
   > On lr, 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