mssql: Improve estimate of max comment length

[jeremy-w]

Before, table comments used a limit of 60 chars (seemingly inherited from the MySQL dialect), while columns warned about exceeding a limit of 255 chars. This led to spurious warnings.

Now, warnings are only emitted if it seems possible for the comment to exceed the actual, documented 7,500 byte limit:

[ @value= ] { 'value'}
Is the value to be associated with the property. value is sql_variant, with a default of NULL. The size of value cannot be more than 7,500 bytes. (sp_addextendedproperty (Transact-SQL), emphasis added)

That limit proved to be when the comment.length exceeded 7500/2, as demonstrated by testing.

(This is a small step towards #4204.)

Questions for Reviewers

• Making the warning more precise:

  • Would it be OK for a schema compiler to run queries to sort out stuff like "what is the database collation"? That would theoretically allow us to actually compute the byte length, not make a pessimistic estimate. (But my guess is "no".)
  • If not, would it be better to use an optimistic estimate of 1 char == 1 byte?

• Should we even bother warning at all? When we get around to actually adding the comments, MSSQL will raise an error if the length is exceeded for us:

  Msg 15097, Level 16, State 1, Server 49df3e21e80c, Procedure sp_updateextendedproperty, Line 16
  The size associated with an extended property cannot be more than 7,500 bytes.

[kibertoad]

@jeremy-w Considering that The size associated with an extended property cannot be more than 7,500 bytes. doesn't mention comments anywhere, warning is still helpful to point people experiencing the issue in the right direction. Could you also add two integration tests, one of them being slightly below the threshold, and another one being above the threshold and actually causing MSSQL to fail?

[jeremy-w]

@kibertoad would the warning still be useful if we trapped the error during adding the comment and converted it into a more knex-relevant error? or would that do to replace the error?

since the "length exceeded" error has Level 16 (greater than 10, less than 20), it looks like try…catch would work.

[kibertoad]

@jeremy-w While we already have plenty of magic for handling MSSQL edge-cases, I would prefer to avoid adding even more unless strictly necessary. Just improving error messages doesn't sound like a big enough reason to complicate generated queries, so I would still prefer just a knex warning in this case.

[jeremy-w]

@kibertoad Updated with the requested tests. Also added a test to validate our "7500/4" estimate for when to warn is a good one.

This also gets us most of the way to being able to generate comments as part of the schema builder, so things are looking good for #4204. :)

[jeremy-w]

thought of a couple issues overnight. i bet we can allow a 2x longer char count before warning due to how JS counts character length when characters outside the BMP get involved.

[jeremy-w]

OK, I addressed the issues I thought of, and I was able to provide a lot more headroom - comments have to be longer than 3,750 characters before comment length begins to be a concern!

[kibertoad]

Thank you!

[kibertoad]

Released in 0.95.3.