dresende/node-sql-query

Postgres "Date" conversion gives errors

Closed this issue · 3 comments

Trying to insert a model with a Date type in Postgres results in the following error:

TypeError: Object Tue May 21 2013 13:12:53 GMT+0200 (CEST) has no method 'replace'
    at Object.exports.escapeVal (/Users/mike/dev/search-analytics/node_modules/orm/node_modules/sql-query/lib/Dialects/postgresql.js:35:19)
    at Object.InsertQuery.build (/Users/mike/dev/search-analytics/node_modules/orm/node_modules/sql-query/lib/Insert.js:26:24)
    at Driver.insert (/Users/mike/dev/search-analytics/node_modules/orm/lib/Drivers/DML/postgres.js:194:21)
    at /Users/mike/dev/search-analytics/node_modules/orm/lib/Instance.js:120:17
    at checkNextValidation (/Users/mike/dev/search-analytics/node_modules/orm/lib/Instance.js:42:12)
    at Instance.handleValidations (/Users/mike/dev/search-analytics/node_modules/orm/lib/Instance.js:66:10)
    at Instance.saveInstance (/Users/mike/dev/search-analytics/node_modules/orm/lib/Instance.js:95:3)
    at Object.Instance.Object.defineProperty.value (/Users/mike/dev/search-analytics/node_modules/orm/lib/Instance.js:336:7)
    at createNext (/Users/mike/dev/search-analytics/node_modules/orm/lib/Model.js:384:19)
    at Function.Model.model.create (/Users/mike/dev/search-analytics/node_modules/orm/lib/Model.js:413:3)

Looking in the source code, it seems that lib/Dialects/postrgresql.js method escapeVal doesn't handle Date objects properly yet.

Modifying the switch statement to look more like this:

switch (typeof val) {
        case "number":
            return val;
        case "boolean":
            return val ? "true" : "false";
        case "object": 
            if (val instanceof Date)
                return "'" + dateToString(val) + "'";
    }

seems to work - but I had to steal dateToString method from the node pg/lib/utils.js. This works but probably isn't the nicest solution so I won't send a PR.

Ideally this library would be a true dependency maybe and you could re-use their methods directly? (doesn't seem like your ORM library should be doing the escaping itself, that seems a little like the job of the core driver maybe?)

Mate - any feedback or can I help on this one?

FYI I worked around it by converting the date to a PG acceptable String in my own code - but that seemed wrong?

I'm going to use your suggestion (and borrow code from pg for now). Thank you for your feedback.

About ORM escaping things, I'm looking to change the drivers in the future to use prepared statements and so avoid escaping both on ORM or node drivers. But for now I'll leave as it is.