mysqljs / sqlstring

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

possibility to ignore values in escape and escapeId #21

Closed SeregPie closed 7 years ago

SeregPie commented 7 years ago

Hello.

Nearly a year ago I created an issue. https://github.com/mysqljs/mysql/issues/1542 Now I take the task in my hand. =)


SqlString.escape({counter: SqlString.escaped('(@var := @var + 1)')});
// => '`counter` = (@var := @var + 1)'

SqlString.escapeId(['id', SqlString.escaped('@col')]);
// => '`id`, @col'
dougwilson commented 7 years ago

Hi @SeregPie I know you just made your PR, but I just got PR #9 landed. Do you think that (1) is this still useful to have, and if so (2) should it be implemented in terms of .toSqlString ?

SeregPie commented 7 years ago

I think toSqlString is just over complicated and does not feel intuitive for me. So i need to create a new object for every value I want to not be escaped.

Your example.


var CURRENT_TIMESTAMP = { toSqlString: function() { return 'CURRENT_TIMESTAMP()'; } };
var sql = SqlString.format('UPDATE posts SET modified = ? WHERE id = ?', [CURRENT_TIMESTAMP, 42]);

Could be just like this.


var sql = SqlString.format('UPDATE posts SET modified = ? WHERE id = ?', [SqlString.escaped('CURRENT_TIMESTAMP()'), 42]);

And you can still use objects if you want, by using the standard toString function.


var CURRENT_TIMESTAMP = { toString: function() { return 'CURRENT_TIMESTAMP()'; } };
var sql = SqlString.format('UPDATE posts SET modified = ? WHERE id = ?', [SqlString.escaped(CURRENT_TIMESTAMP), 42]);
dougwilson commented 7 years ago

Why open a new PR without commenting that in the other PR first? At least right now, I can't merge this due tobmerge conflicts. Can you resolve them?

SeregPie commented 7 years ago

resolved

dougwilson commented 7 years ago

Hi @SeregPie I see you closed this PR. I'm going to land this tonight to get a new version, in which the implementation would be basically what you write here, except with the instanceof checks removed and the class changed to

function Escaped(val) {
  this.toSqlString = function toSqlString() {
    return String(val);
  };
}

Not sure why the closure instead of changing it, and I can commit this under you as the author, but don't want to do that unless you are OK with it. Probably if I don't hear anything in the next 10 hours or so, I'll just commit myself, otherwise let me know if you want to be the author.

SeregPie commented 7 years ago

Sorry. Yes, you can commit it this way. Thank you.