hiddentao/squel

Empty string makes tail comma with .field method

homura opened this issue · 4 comments

homura commented 
squel.select()
  .field('foo')
  .field('')
  .from('bar')
  .toString()
// output: SELECT foo,  FROM bar

v5.10.0
It is a bug?

hiddentao commented

No, this is not a bug. It's intended behaviour since the field value is allowed to be anything, even an SQL function.

homura commented

But

squel.select()
  .field('')
  .field('foo')
  .field('')
  .from('bar')
  .toString()
// output: SELECT foo FROM bar
// this is the way to avoid tail comma now
squel.select()
  .field('')
  .field('foo')
  .from('bar')
  .toString()
// output: SELECT foo FROM bar

Tail comma can lead to errors in many database,such as MySQL,SQLite.
So I think that tail comma should be ignored.
@hiddentao
Would you think about removing tail comma as a new feature in next version?

FerrielMelarpis commented

@homura
The only thing I think this can be used is if you are dynamically passing the field, say its a variable containing the string

let some_field = (some_condition) ? 'col' : ''; 
squel.select()
    .field(some_field)...

otherwise, you can just fix right here if @hiddentao approves of it..
https://github.com/hiddentao/squel/blob/master/src/core.js#L1250

hiddentao commented

Yeah, tail comma could be removed, though it's an exception situation - could you not check the field name you're dynamically setting to ensure it's not empty?