FirebirdSQL / firebird

Firebird server, client and tools
https://www.firebirdsql.org/
1.2k stars 206 forks source link

[FB4] ICU63.1 suppresses conversion errors #8108

Closed dmitry-lipetsk closed 3 days ago

dmitry-lipetsk commented 1 month ago

FB4 returns an empty string when he can't translate Unicode symbol into ICU-codepage.

FB3 in this case returns an error.

Unicode symbol with code 0x115F (input string 'ᅟ')

Connection charset is NONE.


-- FB3 and FB4 are OK.
select cast(_utf8 'ᅟ' as varchar(1) character set utf8) from rdb$database

-- FB3 returns an error, FB4 OK (an error is expected)
select cast(_utf8 'ᅟ' as varchar(1) character set tis620) from rdb$database

-- FB3 and FB4 return an error (it is OK)
select cast(_utf8 'ᅟ' as varchar(1) character set win1251) from rdb$database

The problem in new implementation of callback function UCNV_FROM_U_CALLBACK_STOP in ICU v63.1

https://github.com/unicode-org/icu/blob/5df4d7dfd8d77dd16aa3a0b398d50a22f4c85daa/icu4c/source/common/ucnv_err.cpp#L68-L113


In ICU v52 (FB3) this function does not contain any code

https://github.com/unicode-org/icu/blob/574e7d9d55760680ea14dbfc4908429a58c5d544/icu4c/source/common/ucnv_err.c#L53-L66

asfernandes commented 1 month ago

They are called ignorable code points. So why they must not be ignored?

dmitry-lipetsk commented 1 month ago

I think, we must have one behaviour for built-in and external charsets.

dmitry-lipetsk commented 1 month ago

If you agree with this inconsistent behavior, I can create PR.

asfernandes commented 1 month ago

First, note that there is no relation of FB versions. This relation is specific in Windows due to us deploying a fixed ICU versions. In Linux, we don't deploy ICU library.

So if you're talking about consistent behavior only using ICU (and not modifying it), then a PR may be ok.

dmitry-lipetsk commented 1 month ago

I offer to use a "stable" implementation of callback functions UCNV_TO_U_CALLBACK_STOP and UCNV_FROM_U_CALLBACK_STOP instead standard "mutable" implementations.

It does not require a modification of ICU.

then a PR may be ok.

Ok, I will do it.

dmitry-lipetsk commented 1 month ago

Done. If this patch is ok, I can port it on FB5 and FB6 (master tree).

I decided to do not touch UCNV_TO_U_CALLBACK_STOP.

asfernandes commented 1 month ago

If this patch is ok, I can port it on FB5 and FB6 (master tree).

Please do.