ClojureScript

Instance Reader to Support Micro/Nanoseconds

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • 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")

Activity

Hide
David Nolen added a comment -

Is this behavior supported in Clojure?

Show
David Nolen added a comment - Is this behavior supported in Clojure?
Hide
Jozef Wagner added a comment -

It seems it is

Show
Jozef Wagner added a comment - It seems it is
Hide
Anatoly Polinsky added a comment -

@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 - @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
David Nolen added a comment -

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

Show
David Nolen added a comment - Ok, do you all have a good idea about what a patch would look like?
Hide
Thomas Heller added a comment -

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 - 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.
Hide
Thomas Heller added a comment -

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 - Well, Math. Just realized that this is not really correct, since 1000 is closer to 999 than it is to 100.
Hide
Thomas Heller added a comment -

Sorry, for that brainfart.

Show
Thomas Heller added a comment - Sorry, for that brainfart.
Hide
David Nolen added a comment -

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 - Or we could return a custom ClojureScript Date type that provides the same api as js/Date but adds support for nanoseconds.
Hide
Thomas Heller added a comment -

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 - 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
David Nolen added a comment - - 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 - - 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

Vote (3)
Watch (3)

Dates

  • Created:
    Updated: