CSNW / sql-bricks

Transparent, Schemaless SQL Generation
http://csnw.github.io/sql-bricks
MIT License
204 stars 25 forks source link

Better sql() params support #85

Closed prust closed 8 years ago

prust commented 8 years ago
prust commented 8 years ago

@Suor: This fixes #84. I've updated the documentation (fixing #75), but don't want to submit it until the tests generated from the documentation are passing, which requires a fix for #76. I'd like to also fix #77 while I'm at it (shouldn't be too hard, compared to the other things being done here).

I realize you're relying pretty heavily on sql() as an escape-valve, I'm sorry I lagged for so long on decent params() support for it.

Suor commented 8 years ago

No rush :). I don't use it myself right now, just following my own issues.

prust commented 8 years ago

@Suor: Ok, I addressed #75 and #76 in 180314b (I didn't get to #77).

Here is the relevant snippet from the new docs:

sql(str[, values]) ... Multiple values can be passed in as separate arguments or as an array:

select().where(sql('field @> $', { key: 'value' })).toParams()
// {"text": "SELECT * WHERE field @> $1", "values": [{"key": "value"}]}

Note that numbered params can be used in the literal SQL string and these will be shifted if sql-bricks included numbered parameters earlier in the statement:

select().where({name: 'Fred'}).and(sql('field @> $1', { key: 'value' })).toParams()
// {"text": "SELECT * WHERE name = $1 AND field @> $2", "values": ["Fred", {"key": "value"}]}

I copied your example from #84, but I don't know what the @> operator does and it seems odd to me that the value we're sending in to replace $1 is an object. It might be better if I change the documentation to an example that I understand.

Suor commented 8 years ago

Snippet is a bit misleading, it doesn't show that I can pass several values to sql(). It also doesn't show whether this will work:

select().where({name: 'Fred'})
    .and(sql('f1 @> $2 and f2 @> $1', [{key: 'value' }, {key: 'value2'}])).toParams()

Note that numbers go in reverse.

Also "shifted" is an implementation detail, I think it could be phrased better. Like "Numbered params refer to position in values in current sql() call not globally.

prust commented 8 years ago

@Suor: Thanks for the excellent feedback! I agree on all points. Here's what I did in bbb2a1c:

Here's the full sql() documentation as it stands now (note: I'm guessing that the example is using @> as the jsonb contains operator, not the array contains operator, given the context):

sql(str[, values]) The SQL Bricks namespace (saved to the local variable sql in these docs) can be called as a function to insert SQL into SQL Bricks somewhere that a value is expected (the right-hand side of WHERE criteria, or insert()/update() values):

select('*').from('person').where({'billing_addr_id': sql('mailing_addr_id')})
// SELECT * FROM person WHERE billing_addr_id = mailing_addr_id

Multiple values can be passed in as separate arguments or as an array (this example uses the postgresql jsonb contains operator):

select().where(sql('field @> $ and field @> $', { key: 'value' }, { key: 'value2' })).toParams()
// {"text": "SELECT * WHERE field @> $1 and field @> $2", "values": [{"key": "value"}, {"key": "value2"}]}

Note that numbered params can be used in the literal SQL string and these refer to the position in the current sql() call, not the position in the statement as a whole:

select().where({name: 'Fred'}).and(sql('f1 @> $2 and f2 @> $1', [{key: 'value' }, {key: 'value2'}])).toParams()
// {"text": "SELECT * WHERE name = $1 AND f1 @> $3 and f2 @> $2", "values": ["Fred", {"key": "value"}, {"key": "value2"}]}

Let me know if you have any more feedback, otherwise I'll go ahead and merge this.

Suor commented 8 years ago

Looks good. If you are ok using PostgreSQL operator and explaining it :)