mysqljs / sqlstring

Simple SQL escape and format for MySQL
MIT License
403 stars 78 forks source link

Adding basic support for POINT #44

Closed dresende closed 2 years ago

dresende commented 5 years ago

Does this make sense? I'm overwriting the function locally but perhaps this could land upstream. It's not the most elegant but enables one to insert or update a record with a point, just the way mysql retrieved it.

sqlstring.objectToValues = (obj, tz) => {
    let sql = "";

    for (let key in obj) {
        let val = obj[key];

        if (typeof val === "function") continue;

        if (typeof val == "object" && Object.keys(val).length == 2 && typeof val.x == "number" && typeof val.y == "number") {
            sql += (sql.length === 0 ? "" : ", ") + sqlstring.escapeId(key) + " = " + "GeomFromText('POINT(" + val.x + " " + val.y + ")')";
        } else {
            sql += (sql.length === 0 ? "" : ", ") + sqlstring.escapeId(key) + " = " + sqlstring.escape(val, true, tz);
        }
    }

    return sql;
};

Of course, it only works with 2D points, but for me it's 90% of geometries I use.

dougwilson commented 5 years ago

I'd have to look though the cases, as maybe it would produce some false-positives? What is wrong with using SqlString.raw() or making a Point object with a .toSqlString method on it to pass around?

dresende commented 5 years ago

There are of course false positives. And I just noticed val can be null/undefined and is not checked, but that's easy. What I thought was, the false positive occurrences is extremely tiny (or doesn't make sense at all.

The use case is something like this:

db.query("UPDATE devices SET ? WHERE id = ?", [{
    name     : "John Doe",
    location : { x: -8, y: 40 },
}, device_id, next);
dougwilson commented 5 years ago

Right, I get what you're trying to do :) That's why I'm wondering why should there be any risk of false-positives when there is support for doing this today with no false positives? What, specifically, is wrong with using SqlString.raw() or making a Point object with a .toSqlString method on it to pass around?

dresende commented 5 years ago

It was an easier path, since, when querying the database, the module does not return a Point object. But ok, since I use a small abstraction on top of the module, perhaps I can incorporate a Point there.

dougwilson commented 2 years ago

I'm going to close this as there is a solution with recent versions:

class Point {
  constructor(x, y) {
    this.x = x;
    this.y = y;
  }

  toSqlString() {
    return `GeomFromText('POINT(${SqlString.escape(this.x)} ${SqlString.escape(this.y})')`
  }
}

Eventually the mysql module itself will return objects that have a toSqlString attached to provide built-in round trip support, but that is something that would be a change in that module, rather than here. The pieces are in place in this module these days 👍