Helper function for returning unix_timestamp

Bug #950570 reported by Henrik Ingo
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)
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
Revision history for this message
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  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.