Hi! Can you merge these 3 fixes?

CSNW / sql-bricks

Transparent, Schemaless SQL Generation

http://csnw.github.io/sql-bricks

MIT License

203 stars 25 forks source link

Hi! Can you merge these 3 fixes? #89

Closed vitalif closed 7 years ago

vitalif commented 7 years ago

Change default placeholder for expression parts (my previous pull-request) to be also $%d
Fix .toString() parameter replacer (sql('a = ?', 1).toString({placeholder: '?'}) now crashes, and also does not work with $%d)
Fix merging parameterized sub-expressions (I think select().from('tbl').where(or(sql('a = $1', 444), sql('b = $1', 555))) should be SELECT * FROM tbl WHERE a = $1 OR b = $2, not SELECT * FROM tbl WHERE a = $1 OR b = $1)

vitalif commented 7 years ago

Oops, sorry. The fix was not complete. Please look at the rewritten branch.

prust commented 7 years ago

@vitalif: Thank you! Overall, this looks good & should be merged. There are a few minor clean-up things I would like done, as mentioned in the comments. If you have time & interest you can go ahead & do them, otherwise I'll probably get to them over the weekend or next week and get this merged.

Thanks again!

vitalif commented 7 years ago

Added some updates based on your comments :)

prust commented 7 years ago

Thanks again @vitalif! Merged & published as 1.5.0.