Skip to content
This repository has been archived by the owner on Nov 9, 2021. It is now read-only.

How to register value handler to return non-string (not quoted) value in multi row insert query? #118

Closed
alekbarszczewski opened this issue Jan 14, 2015 · 20 comments

Comments

@alekbarszczewski
Copy link

My goal is to set DEFAULT literal for a column in multi row insert query - tried it like this:

squel = require 'squel'

class Default # empty class

squel.registerValueHandler Default, (val) ->
  'DEFAULT'

q = squel.insert()
q.into('table_name')
q.setFieldsRows [
   { id: 1, col1: 'val1', col2: new Default() }
   { id: 2, col1: 'val2', col2: new Default() }
]
console.log q.toString()

result is:

INSERT INTO table_name (id, col1, col2) VALUES (1, 'val1', 'DEFAULT'), (2, 'val2', 'DEFAULT')

How I can force 'DEFAULT' to not be quoted?

@hiddentao
Copy link
Owner

At the moment there is no elegant way for just DEFAULT to remain unquoted if doing a multi-row insert query. The dontQuote option (if set) will be applied against all values in each row. This clearly sucks, so I think what's needed is the ability to apply options to a given field at a time.

I'll need to fix this. The result will enable you to do something like:

q.setFieldsRows([
   { id: 1, col1: 'val1', col2: new Default() }
   { id: 2, col1: 'val2', col2: new Default() }
], {
  col2: {
    dontQuote: true
  }
})

@alekbarszczewski
Copy link
Author

Thanks for answer. Note that dontQuote option should be available for each row separately because there might be situation like this:

q.setFieldsRows([
   { id: 1, col1: 'val1', col2: new Default() }
   { id: 2, col1: 'val2', col2: 'I want to be quoted!' }
], {
  col2: {
    dontQuote: true
  }
})

Maybe it would be easier to just make it available to somehow mark value as unquotable when returning it from valueHandler? Something like:

squel.registerValueHandler Default, (val, options) ->
   // options will be passed to this function by squel as empty object {}
   options.dontQuote = true
   val

@alekbarszczewski
Copy link
Author

@hiddentao I have created PR with my idea - what do you think?

@derrickpelletier
Copy link

looking for this feature as well

@hiddentao
Copy link
Owner

@alekbarszczewski Please see my comments in #119

@hiddentao hiddentao added this to the 3.11.0 milestone Jan 28, 2015
@hiddentao hiddentao modified the milestone: 3.11.0 May 23, 2015
@hiddentao
Copy link
Owner

The new fval() method in squel 3.11.0 solves this problem:

squel = require 'squel'

q = squel.insert()
q.into('table_name')
q.setFieldsRows [
   { id: 1, col1: 'val1', col2: squel.fval('DEFAULT') }
   { id: 2, col1: 'val2', col2: squel.fval('DEFAULT') }
]
console.log q.toString()   

# INSERT INTO table_name (id, col1, col2) VALUES 
    (1, 'val1', (DEFAULT)) 
    (2, 'val2', (DEFAULT)) 

@abgivant
Copy link

The fval() function doesn't seem to be present in newer versions of the library. Did this get left out accidentally, or was this intentionally removed?

Also, in the version I'm testing with (4.0.0), fval() works when using it in a call to set(), but it doesn't seem to work when used inside of a custom value handler. A while ago I was looking for something that would allow me to do this. Would it be possible to re-add this function and allow it to work within value handlers?

@hiddentao
Copy link
Owner

fval() is now str(). How are you trying to use it in a custom value handler? What's your code?

@abgivant
Copy link

I just updated to the latest version and tried the str() method and it seems to be working perfectly! I just kept missing that when I was looking at the docs. Thanks!

Just to give an example, though, here's where I'm using that.

// Set up a custom value handler for dates
squel.registerValueHandler(Date, function(date) {
    if (date.getTime() === constants.DateKeywords.Now.getTime()) {
        return squel.str('utc_timestamp()');
    }

    if (date.getTime() === constants.DateKeywords.Never.getTime()) {
        return '9999-9-9';
    }

    return utils.getUTCTimestamp(date);
});

@abgivant
Copy link

Actually I just did some more testing with this, and it looks like now everything that is returned from a custom value handler is inserted into the final query unquoted. Is this expected? This did not used to be the case with 3.10.

This handler:

squel.registerValueHandler(Date, function(date) {
  return 'utc_timestamp()';
});

will now return an unquoted string even without a call to squel.str().

squel.insert()
  .into('table')
  .set('col', new Date())
  .toString();

// Generated Query
"INSERT INTO table (col) VALUES ((utc_timestamp()))"

@hiddentao
Copy link
Owner

Yes, this is expected. This is so that you have more flexibility over how the custom value output gets rendered within the final query. If you want it quoted you can now add them yourself.

@abgivant
Copy link

Ah okay. That makes sense.

However, I noticed that this isn't the case for all value handlers. A value returned from a string value handler will still be quoted, but numbers and Dates will not unless it is done explicitly. How is it determined which types get auto-quoted and which don't? And is this something that could be added to the docs?

@hiddentao
Copy link
Owner

@abgivant That's interesting. I thought none of the returned values get quoted. I just pushed 5.4.0 which slightly modifies how it detects if custom value formatting took place - would you be able to try again and let me know if this fixes that discrepancy you're seeing?

@abgivant
Copy link

I just tested the changes a bit more and it seems to be consistently returning everything unquoted now, unless done explicitly. Thanks for the quick update!

However, a change like this could potentially cause a lot of problems if/when someone updates to the newer versions, so would it be possible to have a statement at the top of the "Custom value types" section of the docs specifically describing this behavior? It's implied from the examples, but I think stating it would still be beneficial.

@hiddentao
Copy link
Owner

Updated the docs for custom value types with some info on quoting. Not super prominent but it's in there.

@abgivant
Copy link

abgivant commented Sep 2, 2016

Just ran into another issue with this. It looks like registering a string value handler overrides the dontQuote option if it is specified on a query.

squel.registerValueHandler('string', function(str) {
  return "'" + str + "'";
});
squel.insert().into('table').set('col', 'blah', {dontQuote: true}).toString()
> "INSERT INTO table (col) VALUES (('blah'))"

I can see how these can be conflicting, but what do you think about dontQuote taking precedence over any quoting that is done in a custom value handler?

@hiddentao
Copy link
Owner

@abgivant Sorry about the late reply. If dontQuote were to take precendence it would mean scanning the value returned from the custom value handler in order to trim its quotes - this doesn't make sense to me since it then makes the quoting redundant.

On the other hand, it would be good if the custom value handlers had more information on the context in which they're running in order to know whether to add quotes or not. I'm going to see if that can be made possible.

@abgivant
Copy link

abgivant commented Sep 16, 2016

I don't think it'd be that weird, to strip the quotes if dontQuote was specified. That doesn't seem much different that what happens by default when custom value handlers aren't specified. The actual query declaration logic is going to be more specific to a situation than custom value handlers, so I'd think overrides specified at that level should take precedence.

For example, in the case where I ran into this, I have a string value handler that does some extra escaping before saving the string, but now anywhere I'm trying to insert using a SQL function (utc_timestamp() in this case) fails because of the quotes I'm having to manually add to the string after escaping it in the value handler.

Allowing the value handlers to be aware of the specified options would solve the problem as well, though.

@hiddentao
Copy link
Owner

Allowing the value handlers to be aware of the specified options would solve the problem as well, though.

I like this idea. Will add it into the next release.

@hiddentao
Copy link
Owner

In master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants