Skip to content

Timestamp with timezone fails to parse minutes - #309

Closed
ajitk wants to merge 1 commit into
brianc:masterfrom
ajitk:master
Closed

Timestamp with timezone fails to parse minutes#309
ajitk wants to merge 1 commit into
brianc:masterfrom
ajitk:master

Conversation

@ajitk

@ajitk ajitk commented Mar 17, 2013

Copy link
Copy Markdown

The timezone parser in text parser modules missed the : that appears in timezone. As a result it failed to parse the minutes component of the timezone.

@brianc

brianc commented Mar 17, 2013

Copy link
Copy Markdown
Owner

Could you whip up a regression test for this? Somewhere around here would be good:

https://github.com/brianc/node-postgres/blob/master/test/unit/client/typed-query-results-tests.js#L86

@ajitk

ajitk commented Mar 17, 2013

Copy link
Copy Markdown
Author

The test 'timestamptz with minutes in timezone' is already present. Should the value of property actual be '2010-10-31 14:54:13.74-05:30' instead of '2010-10-31 14:54:13.74-0530' in this test?

@brianc

brianc commented Mar 17, 2013

Copy link
Copy Markdown
Owner

The test is passing for me before I merge this pull request so it's either not testing the right thing, or the test is incorrect.

@ajitk

ajitk commented Mar 18, 2013

Copy link
Copy Markdown
Author

I am certain about following, pg returns timezone in hh:mm format.

test=> create table test(ts timestamptz default current_timestamp);
CREATE TABLE
test=> insert into test(ts) values(default) returning *;
            ts                
----------------------------------
 2013-03-18 10:25:00.177743+05:30
(1 row)

INSERT 0 1
test=> 

Timezone input in postgres is more lenient as mentioned in the docs: http://www.postgresql.org/docs/9.2/static/datatype-datetime.html#DATATYPE-DATETIME-INPUT. I did not find any reference to the timezone output format.

http://www.w3.org/TR/NOTE-datetime document however is clear about the formats. It specifies TZD = time zone designator (Z or +hh:mm or -hh:mm).

@brianc brianc closed this in 2ef1bbf Apr 22, 2013
@brianc

brianc commented Apr 22, 2013

Copy link
Copy Markdown
Owner

@ajitk I appreciate your help with this one. Sorry it took so long. I was confused on the failing tests, but I got it sorted out now.

@ajitk

ajitk commented Apr 22, 2013

Copy link
Copy Markdown
Author

@brianc Thanks for the great work. You rock!

@brianc

brianc commented Apr 22, 2013

Copy link
Copy Markdown
Owner

😄 open source rocks! 🎸 💃 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants