Helper function for returning unix_timestamp

Bug #950570 reported by Henrik Ingo on 2012-03-09
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Drizzle
In Progress
Wishlist
Unassigned

Bug Description

Item objects keep time information in a format inherited from MySQL. If you want to get that time as a unix timestamp you end up writing quite many lines of code. It would be convenient to have this as a utility function so you don't have to copy the code every time you need to do this conversion. (For instance, it could be a method of the Item class(es).

For an example of how unix timestamp is computed, see

drizzled/function/time/unix_timestamp.cc

...which provides the functionality to call UNIX_TIMESTAMP() in SQL. This issue is a feature request to provide a utility function for Drizzle internal use.

An example consumer of this utility function is found in plugin/js/js.cc:195:

        // DATE/TIME values are also STRING_RESULT, make them a Date type in v8
        // Now we need to get the unix timestamp integer, surprisingly tricky...
        // TODO: This should really be just args[n]->get_epoch_seconds(). I need to write a separate patch for Item class one of these days.
        type::Time ltime;
        Timestamp temporal;
        args[n]->get_date(ltime, 0);
        temporal.set_years(ltime.year);
        temporal.set_months(ltime.month);
        temporal.set_days(ltime.day);
        temporal.set_hours(ltime.hour);
        temporal.set_minutes(ltime.minute);
        temporal.set_seconds(ltime.second);
        temporal.set_epoch_seconds();
        if (temporal.is_valid())
        {
          time_t tmp;
          temporal.to_time_t(tmp);
          // Pay attention, Ecmascript defines a date as *milliseconds* since unix epoch
          // Also, on platforms where time_t is 32 bit, we need explicit cast to 64 bit integer
          a->Set( n-1, v8::Date::New(((uint64_t)tmp)*1000) );
        } else {
          a->Set( n-1, v8::String::New(args[n]->val_str(str)->c_str() ) );
        }

It should be possible to simply replace this code with a

        if ( time_t tsamp = args[n]->get_epoch_seconds() )
        {
          // Pay attention, Ecmascript defines a date as *milliseconds* since unix epoch
          // Also, on platforms where time_t is 32 bit, we need explicit cast to 64 bit integer
          a->Set( n-1, v8::Date::New(((uint64_t) tstamp )*1000) );
        } else {
          a->Set( n-1, v8::String::New(args[n]->val_str(str)->c_str() ) );
        }

Henrik Ingo (hingo) on 2012-03-09
Changed in drizzle:
importance: Undecided → Wishlist
status: New → Confirmed
tags: added: low-hanging-fruit refactor
Changed in drizzle:
assignee: nobody → M.Sharan Kumar (sharan-monikantan)
status: Confirmed → In Progress
description: updated
Changed in drizzle:
assignee: M.Sharan Kumar (sharan-monikantan) → nobody
Rob Smith (kormoc) wrote :

I believe the branch I just related to fixes this issue ( lp:~kormoc/drizzle/950570 ). If someone wants to take a look and give me feedback, that'd be awesome.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers