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

Add POINT(X, Y) support #8

Closed
wants to merge 3 commits into from
Closed

Conversation

kevinmartin
Copy link
Contributor

@kevinmartin kevinmartin commented Aug 24, 2016

Adds POINT(x, y) support. Assumes an object with constructor name 'Point'.

Can be easily added like so:

// Simple:
function Point(x, y) {
    this.x = x;
    this.y = y;
}

// Verbose (ES6):
class Point {
    constructor(x, y) {
        this.x = x;
        this.y = y;
    }
}

// Escaping:
SqlString.escape(new Point(123, 456)); // -> POINT(123, 456)

I wrote it this way since, judging from the conversation in mysqljs/mysql#877, almost anything could break SqlString.objectToValues(...).

@kevinmartin
Copy link
Contributor Author

kevinmartin commented Aug 25, 2016

Maybe instead of a BYO Point Class, it can be included in the library as SqlString.Point(x, y)?

@dougwilson dougwilson self-assigned this Aug 26, 2016
@dougwilson
Copy link
Member

Yea, this is a much better implementation than the other pull request. I'm wondering if we may actually be better served by looking for a .toSQL method on an object and calling that (or .toMySQL() ? .toSQL('mysql')?) and letting the object generate whatever SQL it feels is most appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants