[Asterisk-code-review] func_odbc: acf_odbc_read() and cli_odbc_read() unicode support (...asterisk[master])

[Sean Bright]

Sean Bright has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/12812 )

Change subject: func_odbc:  acf_odbc_read() and cli_odbc_read() unicode support
......................................................................


Patch Set 4:

> Patch Set 4:
> 
> > Patch Set 4:
> > 
> > > Patch Set 4:
> > > 
> > > > Patch Set 4:
> > > > 
> > > > I think that the correct fix is going to be to modify ast_odbc_ast_str_SQLGetData() to retrieve the data in parts, as documented here:
> > > > 
> > > > https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgetdata-function?view=sql-server-2017#retrieving-variable-length-data-in-parts
> > > 
> > > How about a fixed-length data in parts (ex. NCHAR)? Microsoft prohibits to retrieve the data in parts for a fixed-length data.
> > 
> > I think your original solution may be the best (except you should multiply by 4 instead of 3).
> > 
> > The problem is that the drivers are inconsistent and I'm sure they are not even consistent with themselves between releases. When using the "octet length" check, MySQL returns a value that is sufficient to store the bytes in UTF-8, while Microsoft's does not. Multiplying the number of potential characters - while not ideal - is a better solution than potentially adding different logic for each of the ODBC drivers.
> > 
> > Because UTF-8 can require up to 4 bytes per codepoint, I think we can consider the 4x factor as the worst-case scenario for Unicode data.
> > 
> > Alexei - thoughts?
> 
> I think need to add configuration option to res_odbc.conf to deal with Microsoft issue.
> If option is enabled them 4x, otherwise "octet length". By default disabled.
> The MSSQL Server 2019 supports UTF-8.

I don't personally feel that it is worth it, but I will bow out of the discussion because I don't know enough about how to deal with Unicode & ODBC.


-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/12812
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I50e86c8a277996f13d4a4b9b318ece0d60b279bf
Gerrit-Change-Number: 12812
Gerrit-PatchSet: 4
Gerrit-Owner: Boris P. Korzun <drtr0jan at yandex.ru>
Gerrit-Reviewer: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Boris P. Korzun <drtr0jan at yandex.ru>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-CC: Sean Bright <sean.bright at gmail.com>
Gerrit-Comment-Date: Sun, 08 Sep 2019 16:23:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190908/e5e302af/attachment.html>