Skip to content
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

Support for time INTERVAL data type in postgres #4900

Open
mark-lester opened this issue Nov 20, 2015 · 36 comments
Open

Support for time INTERVAL data type in postgres #4900

mark-lester opened this issue Nov 20, 2015 · 36 comments
Assignees
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@mark-lester
Copy link

I have times > 24 hours, which just broke upon moving to postrgres, so I think I need to implement an INTERVAL data type in data-types.js.

@mark-lester mark-lester changed the title Support for time INTERVAL data type in postgress Support for time INTERVAL data type in postgres Nov 20, 2015
@mark-lester
Copy link
Author

I added the obvious

/**
 * A time interval column
 * @property INTERVAL
 */

var INTERVAL = function() {
  if (!(this instanceof INTERVAL)) return new INTERVAL();
  ABSTRACT.apply(this, arguments);
};
util.inherits(INTERVAL, ABSTRACT);

INTERVAL.prototype.key = INTERVAL.key = 'INTERVAL';
INTERVAL.prototype.toSql = function() {
  return 'INTERVAL';
};

and the entry in module.exports.
Unless I get told otherwise I'll submit that shortly. I cant fully test yet as I have a weird bug on constraints and block inserts (it goes away when I bring the block size down so I need to have a proper look at what is being said to postrgres)

@mickhansen mickhansen added the type: feature For issues and PRs. For new features. Never breaking changes. label Nov 21, 2015
@mickhansen
Copy link
Contributor

The newer version uses ABSTRACT.inherts(cb) afaik, but @janmeier is more knowledgeable here.

@janmeier
Copy link
Member

Have a look at https://github.com/sequelize/sequelize/blob/master/lib/dialects/postgres/data-types.js for an example of how to extend the abstract datatype.

You'll need to add an entry in https://github.com/sequelize/sequelize/blob/master/lib/data-types.js as well, like with hstore and json, otherwise the type won't be accessible

@janmeier
Copy link
Member

janmeier commented Dec 4, 2015

ping @mark-lester :)

@mark-lester
Copy link
Author

Hey. where did I get to ?, I think I gave up, yes, I aborted and went for strings, because ...,
https://groups.google.com/forum/#!topic/sequelize/IBoT8wf72tg
I am just in the middle of critical phase in today's cooking, let me remember why I bottled out, ..., brb

@janmeier
Copy link
Member

janmeier commented Dec 4, 2015

While other dialects use strings to identify their types (e.g. INTEGER, GEOMETRY etc), postgres uses oids, which are numbers. The oid for interval is 1186, and interval[] is 1187. Most oids are static, but those for user created types / types you have to manually enable, fx geometry and hstore are dynamic.

(You can check for an oid using `select typname, oid, typarray from pg_type where typtype = 'b')

@mark-lester
Copy link
Author

ok, thanks,
I am about to return to the server and so will have another punt, I need to to implement a sequence manager, something that does this kind of thing (all totally untested).

sequenceManage:function(offset){
    var self=this;
    var query = "UPDATE "+this.tableName+
        " SET "+this.sequenceName+" = "+this.sequenceName+"+"+offset+
        " WHERE "+this.sequenceName +" >= "+this.get(this.sequenceName);
    _.each(this.seqIndexes,function(seqIndex){
        query+=" AND "+seqIndex+"="+self.get(seqIndex);
    });
    return this.sequelize.query(query);
},

and this

            modelDef.hooks={
                beforeInsert: function(record, options) {
                    this.sequenceManage(1);
                },
                    afterDelete: function(record, options) {
                    this.sequenceManage(-1);
                }
            },

and obviously give your def a sequenceName (name of field that holds the sequence) and sequencIndex[], list of fields to sequence across.
If there's a thing to do that pray tell, but I will go back now and attempt to return to using INTERVAL

@mark-lester
Copy link
Author

I need guidance.
I am trying to bend an INTERVAL into a timey thing that returns a string of the form H:m:s
I made this parse method, but to no avail, interval is a "H:m:s" string already, but I am getting an object from my query .

 INTERVAL.parse  = INTERVAL.prototype.parse=function(interval) {
    _.each( ['hours','minutes','seconds'],function(arg){
       if (interval[arg].length === 1)
          interval[arg]="0"+interval[arg];
    });

    return interval.hours+":"+interval.minutes+":"+interval.seconds;
  };

any clues on where I am supposed to stop an INTERVAL field from returning an object consisting of hours,minutes and seconds.,
and is this what we really want anyway ?. The object form might be what we should have. What I myself want is just an H:m:s string where H can be > 24. is that fair ?.

@janmeier
Copy link
Member

janmeier commented Dec 7, 2015

I don't have any strong feelings about the returned type for interval, since you are the one implementing it, feel free to write it to suit your needs.

Adding a .parse method should be all that's required - Is that method being called at all? Perhaps you can create a pull request with what you have so far, that would make it easier for me to offer some guidance

@mark-lester
Copy link
Author

OK, I let this slip as I went off back into the client. I have some other stuff now, namely I want to do things like select "name",earth_distance (ll_to_earth(88.9761,24.621281) , ll_to_earth("lon","lat")) from "Stops"; so that would be a cool thing to have.
I did some work previously on epilogue so it could do aggregates like max and min, and you can append $lt etc so you can do afield$lt=, so I got near to this stuff, at least using the interface.
I'll have another look first and then post an issue. If someone has done any gis stuff and I just cant see it please say.

@stale stale bot added the stale label Jun 29, 2017
@stale stale bot closed this as completed Jul 6, 2017
@dskrvk
Copy link

dskrvk commented Sep 25, 2017

My team is interested in the INTERVAL type as well.

@ClarenceL
Copy link

So unsupported until further notice? Would like this as well.

@abelosorio
Copy link

Another vote here. Please support the INTERVAL type.

@yussinsharp
Copy link

yussinsharp commented Dec 5, 2017

+1 Support for INTERVAL would be very helpful for my team as well.

@sushantdhiman sushantdhiman added dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). and removed stale labels Dec 14, 2017
@sushantdhiman sushantdhiman reopened this Dec 14, 2017
@friend0
Copy link

friend0 commented Feb 9, 2018

another interval support request

@drico

This comment was marked as spam.

@hazrul
Copy link

hazrul commented Jun 13, 2018

another interval support request

@abelosorio
Copy link

abelosorio commented Jul 9, 2018

Hi everyone! I really need this feature. I've been waiting for this a long time (I didn't have the time to work on it) and today I decided to spend some time trying to add it.

I wrote it using @mark-lester's suggestions and @janmeier's comments.

It's very simple. It does not perform any parsing PostgreSQL <-> Sequelize. Values are read and returned as strings.

Example table:

                                                      Table "public.facilities"
  Column  |          Type          |                        Modifiers                        | Storage  | Stats target | Description 
----------+------------------------+---------------------------------------------------------+----------+--------------+-------------
 id       | integer                | not null default nextval('facilities_id_seq'::regclass) | plain    |              | 
 name     | character varying(255) | not null                                                | extended |              | 
 tzOffset | interval               |                                                         | plain    |              | 

Example record creation:

models.facility.create({ name: 'Abel\'s house', tzOffset: '-3 hours' });

Example query:

models.facility.findById(2);
// {
//   "id": 2,
//   "name": "Abel's house",
//   "tzOffset": "-03:00:00"
// }

This is pretty much all I need so far.

Let me know if it is good enough for everyone and I can continue working on my PR in order to having this ready to merge ASAP.

Thanks in advance.

cc @sergioshev

@abelosorio
Copy link

ping @mickhansen @janmeier

Sorry for bothering you, I'm sure you have plenty of work going on, but I really need this feature and I need basic directions to keep working on this.

My PR contains the basic functionally, but if you are OK with this approach I'll add more tests and all it needs to be merged.

Thanks for your time!

@abelosorio
Copy link

Temporary solution 👉 https://www.npmjs.com/package/sequelize-interval-postgres

@StefanFeederle
Copy link

My team is interested in the INTERVAL type as well.

@gabegorelick
Copy link
Contributor

See rails/rails#16919 for INTERVAL support in Rails. Lots of interesting stuff there.

Some question for any potential solution:

  • How are intervals returned to the caller? PostgresInterval objects (this is what node-posgres uses under the hood), moment.duration objects, decimals, or strings?
  • What representations are accepted from the caller?
  • How are strings parsed? Is ISO format handled? Is PG's custom format handled? For input and output? How is intervalstyle managed?

@abelosorio Seems like your solution just passes the results directly to/from pg? Is that correct?

@gabegorelick
Copy link
Contributor

I've attempted a solution here: #10441

Caveats:

  • Requires ISO intervalstyle outputs
  • Represents intervals as moment.duration objects

@abelosorio
Copy link

Hi @gabegorelick! Great to see someone else worried about this issue.

My solution passes the value direct to PostgreSQL, yes. When it reads it, it returns an object. This is because the value is parsed by postgres-interval.

This object is completely compatible to the Duration class from the Moment library, so you can use moment.duration(value) to convert the value to a Duration instance.

Let me know if I can help further. It's been a while since the last time I worked on this issue, but I'm glad you are now.

@gabegorelick
Copy link
Contributor

gabegorelick commented Feb 13, 2019

Thanks @abelosorio. I've updated #10441 to support PgInterval objects. They'll be converted to moment.duration objects, which are then converted to ISO strings before being sent to PG.

By the way, since postgres-interval doesn't support non-default intervalstyles, what happens when you use your plugin with other intervalstyles? Does postgres-interval get confused?

I've worked around this by setting intervalstyle to iso_8601 when we grab a connection. But I wish there was a better solution.

@abelosorio
Copy link

abelosorio commented Feb 13, 2019 via email

@gunar
Copy link

gunar commented Feb 14, 2019

Thank you for your work @abelosorio @gabegorelick . Excited about getting this merged.

@abelosorio
Copy link

Yeah... so do I. Finally closing a 3-year issue 💪

@gabegorelick
Copy link
Contributor

If you'd like to help out, please provide feedback on #10441. There are still some open questions that need to be answered about the design of that approach.

@stale

This comment was marked as outdated.

@stale stale bot added the stale label Jul 23, 2019
@papb papb removed the stale label Jul 24, 2019
@gabegorelick
Copy link
Contributor

Still an issue. I will rebase #10441 when I get a chance.

@jbdevelop
Copy link

jbdevelop commented Jan 28, 2021

I was trying to insert data...

I got it just like that

await TransferDevice.create({
          device_id: item.device_id,
          transfer_id: item.transfer_id,
          transfer_route_id: item.transfer_route_id,
          start_date: null,
          end_date: null,
          last_update: null,
          tracking_token: null,
          created_at: Sequelize.literal(`NOW() - INTERVAL '3h'`),
          updated_at: Sequelize.literal(`NOW() - INTERVAL '3h'`),
        }) 

https://stackoverflow.com/questions/43634819/how-to-use-interval-in-sequelizejs

@sfroskos
Copy link

Are PostgreSQL intervals now supported in sequelize? Is there any guidance on how to do create a model and query it using intervals?

@jthoward64
Copy link

Just for anyone who comes looking, I think this issue is essentially blocked behind the new temporal support (#14295)

@ephys
Copy link
Member

ephys commented Jun 14, 2023

I think so too. I'd prefer to map the interval data type to Temporal.Duration (rather than our own representation), as they're both ISO 8601 compatible

@jthoward64
Copy link

I think so too. I'd prefer to map the interval data type to Temporal.Duration (rather than our own representation), as they're both ISO 8601 compatible

On that, I have been using a custom data type in the meantime that others may find helpful, it uses Luxon's duration under the hood. I haven't tested it exhaustively, but I haven't run into issue so far.

import { DataTypes } from "@sequelize/core";
import type { DurationLikeObject } from "luxon";
import { Duration } from "luxon";

export class DurationDataType extends DataTypes.ABSTRACT<Duration> {
  toSql() {
    return "interval";
  }

  validate(value: unknown): value is Duration {
    return value instanceof Duration;
  }

  areValuesEqual(
    value: Duration | null | undefined,
    originalValue: Duration | null | undefined
  ): boolean {
    return value == null || originalValue == null
      ? value === originalValue
      : value.equals(originalValue);
  }

  escape(value: Duration): string {
    const stringified = value.toISO();
    if (stringified == null) {
      throw new Error("Could not serialize Duration to ISO string");
    }
    return stringified;
  }

  toBindableValue(value: Duration): string {
    return this.escape(value);
  }

  parseDatabaseValue(value: unknown): unknown {
    try {
      if (typeof value === "string") {
        return Duration.fromISO(value);
      }
      if (typeof value === "number") {
        return Duration.fromMillis(value);
      }
      if (typeof value === "object") {
        return Duration.fromObject(value as DurationLikeObject);
      }
      throw new Error("Could not parse Duration from database value");
    } catch (error) {
      const message = error instanceof Error ? error.message : "Unknown error";
      throw new Error(
        `Could not parse Duration from database value: ${message}`
      );
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.