Skip to content

Timestamp with timezone fails to parse minutes #309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

ajitk
Copy link

@ajitk ajitk commented Mar 17, 2013

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
Copy link
Owner

brianc commented Mar 17, 2013

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
Copy link
Author

ajitk commented Mar 17, 2013

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
Copy link
Owner

brianc commented Mar 17, 2013

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
Copy link
Author

ajitk commented Mar 18, 2013

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
Copy link
Owner

brianc commented Apr 22, 2013

@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
Copy link
Author

ajitk commented Apr 22, 2013

@brianc Thanks for the great work. You rock!

@brianc
Copy link
Owner

brianc commented Apr 22, 2013

😄 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