[#CLJS-454] Instance Reader to Support Micro/Nanoseconds - Clojure JIRA

Details

Type: Defect
Status: Open
Priority: Major
Resolution: Unresolved
Affects Version/s: None
Fix Version/s: None
Component/s: None
Labels:
- reader
- timestamp
Environment:
N/A

Description

Any timestamps with a greater than millisecond precision cannot be handled by the ClouseScript reader:

> cljs.reader.read_date("2012-12-30T23:20:05.066980000-00:00")
> Error: Assert failed: timestamp millisecond field must be in range 0..999 Failed:  0<=66980000<=999 (<= low n high)

Here "2012-12-30T23:20:05.066980000-00:00" is an example of an ordinary timestamp that is returned from Postgres.

ClojureScript reader interprets the nanosecond portion "066980000" as milliseconds and the check here fails:

def parse-and-validate-timestamp
...
   (check 0 ms 999 "timestamp millisecond field must be in range 0..999")

More Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

cljs-reader-nanosecond-workarround-corrected.patch

05/Feb/13 3:37 PM

1 kB

Thomas Heller

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

David Nolen added a comment - 04/Jan/13 4:55 PM

Is this behavior supported in Clojure?

Show

David Nolen added a comment - 04/Jan/13 4:55 PM Is this behavior supported in Clojure?

Hide

Permalink

Jozef Wagner added a comment - 05/Jan/13 6:48 AM

It seems it is

Show

Jozef Wagner added a comment - 05/Jan/13 6:48 AM It seems it is

Hide

Permalink

Anatoly Polinsky added a comment - 06/Jan/13 2:18 AM

@David,

Yep, it is supported. Enhancing Jozef's response:

user=> (use 'clojure.instant)
nil
user=> (read-instant-date "2012-12-30T23:20:05.066980000-00:00")
#inst "2012-12-30T23:20:05.066-00:00"

/Anatoly

Show

Anatoly Polinsky added a comment - 06/Jan/13 2:18 AM @David, Yep, it is supported. Enhancing Jozef's response:

user=> (use 'clojure.instant)
nil
user=> (read-instant-date "2012-12-30T23:20:05.066980000-00:00")
#inst "2012-12-30T23:20:05.066-00:00"

/Anatoly

Hide

Permalink

David Nolen added a comment - 09/Jan/13 10:59 PM

Ok, do you all have a good idea about what a patch would look like?

Show

David Nolen added a comment - 09/Jan/13 10:59 PM Ok, do you all have a good idea about what a patch would look like?

Hide

Permalink

Thomas Heller added a comment - 05/Feb/13 3:28 PM

Since JavaScript has no support for nanoseconds in Date, I'd vote for dropping the nanoseconds. Currently the reader just blows up when reading a String with java.sql.Timestamp printed in CLJ, since that is printed in nanosecond precision.

While probably not the best solution, I attached a patch for cljs.reader/parse-and-validate-timestamp fn that just drops the nanoseconds from the millisecond portion. Would be easier to just strip the ms string to 3 digits but that would cause "2012-12-30T23:20:05.066980000012312312123121-00:00" to validate.

Show

Thomas Heller added a comment - 05/Feb/13 3:28 PM Since JavaScript has no support for nanoseconds in Date, I'd vote for dropping the nanoseconds. Currently the reader just blows up when reading a String with java.sql.Timestamp printed in CLJ, since that is printed in nanosecond precision. While probably not the best solution, I attached a patch for cljs.reader/parse-and-validate-timestamp fn that just drops the nanoseconds from the millisecond portion. Would be easier to just strip the ms string to 3 digits but that would cause "2012-12-30T23:20:05.066980000012312312123121-00:00" to validate.

Thomas Heller made changes - 05/Feb/13 3:29 PM

Field	Original Value	New Value
Attachment		cljs-reader-nanosecond-workarround.patch [ 11837 ]

Hide

Permalink

Thomas Heller added a comment - 05/Feb/13 3:35 PM

Well, Math. Just realized that this is not really correct, since 1000 is closer to 999 than it is to 100.

Show

Thomas Heller added a comment - 05/Feb/13 3:35 PM Well, Math. Just realized that this is not really correct, since 1000 is closer to 999 than it is to 100.

Hide

Permalink

Thomas Heller added a comment - 05/Feb/13 3:37 PM

Sorry, for that brainfart.

Show

Thomas Heller added a comment - 05/Feb/13 3:37 PM Sorry, for that brainfart.

Thomas Heller made changes - 05/Feb/13 3:37 PM

Attachment

cljs-reader-nanosecond-workarround-corrected.patch [ 11838 ]

Hide

Permalink

David Nolen added a comment - 06/Feb/13 11:39 AM

Or we could return a custom ClojureScript Date type that provides the same api as js/Date but adds support for nanoseconds.

Show

David Nolen added a comment - 06/Feb/13 11:39 AM Or we could return a custom ClojureScript Date type that provides the same api as js/Date but adds support for nanoseconds.

Thomas Heller made changes - 12/Feb/13 6:15 AM

Attachment

cljs-reader-nanosecond-workarround.patch [ 11837 ]

Hide

Permalink

Thomas Heller added a comment - 20/Feb/13 5:20 PM

One could extend the js/Date prototype with a setNanos/getNanos method and call them accordingly. I'd offer to implement that but the cljs.reader/parse-and-validate-timestamp function scares me, any objections to rewriting that?

As for the js/Date extension, thats easy enough:

(set! (.. js/Date -prototype -setNanos) (fn [ns] (this-as self (set! (.-nanos self) ns))))
(set! (.. js/Date -prototype -getNanos) (fn [] (this-as self (or (.-nanos self) 0))))

Not sure if its a good idea though, messing with otherwise native code might not be "stable".

Not convinced that keeping the nanos around is "required" since javascript cannot construct Dates with nanos, but its probably better not to lose the nanos in case of round tripping from clj -> cljs -> clj.

Show

Thomas Heller added a comment - 20/Feb/13 5:20 PM One could extend the js/Date prototype with a setNanos/getNanos method and call them accordingly. I'd offer to implement that but the cljs.reader/parse-and-validate-timestamp function scares me, any objections to rewriting that? As for the js/Date extension, thats easy enough: (set! (.. js/Date -prototype -setNanos) (fn [ns] (this-as self (set! (.-nanos self) ns)))) (set! (.. js/Date -prototype -getNanos) (fn [] (this-as self (or (.-nanos self) 0)))) Not sure if its a good idea though, messing with otherwise native code might not be "stable". Not convinced that keeping the nanos around is "required" since javascript cannot construct Dates with nanos, but its probably better not to lose the nanos in case of round tripping from clj -> cljs -> clj.

Hide

Permalink

David Nolen added a comment - 21/Feb/13 12:25 AM - edited

We definitely don't want to mutate Date's prototype without namespacing. I'm not sure we want to mutate Date's prototype at all. That's why I suggested a ClojureScript type with the same interface as Date just as Google has done in Closure.

Show

David Nolen added a comment - 21/Feb/13 12:25 AM - edited We definitely don't want to mutate Date's prototype without namespacing. I'm not sure we want to mutate Date's prototype at all. That's why I suggested a ClojureScript type with the same interface as Date just as Google has done in Closure.

People

Assignee:

Unassigned

Reporter:

Anatoly Polinsky

Vote (4)

Watch (4)

Dates

Created:

04/Jan/13 3:30 PM

Updated:

21/Feb/13 1:22 PM