tds-fdw / tds_fdw

A PostgreSQL foreign data wrapper to connect to TDS databases (Sybase and Microsoft SQL Server)
Other
381 stars 102 forks source link

Character set conversion problems should issue a warning but not terminate the query #351

Open cstork opened 10 months ago

cstork commented 10 months ago

I have a Sybase DB (aka SAP SQL Anywhere) that contains text columns that in very rare cases contain invalid characters (or just null characters, which Postgres does not allow, since it uses cstrings to store text). When tds_fdw accesses this table my query fails with:

ERROR: DB-Library error: DB #: 2403, DB Msg: Some character(s) could not be converted into client's character set. Unconverted bytes were changed to question marks ('?'), OS #: 0, OS Msg: Success, Level: 4

It seems that freeTDS's inconv routine "fixes" the text while issuing a warning but tds_fdw makes a fatal error out of it – an error caused by bad data that is essentially out of the control of the client.

Wouldn't it be better to make character set conversion errors non-fatal and to use the already converted string with question marks?

(BTW, if somebody could point me to the place in the code where the error manifests, I'd be grateful.)

GeoffMontee commented 10 months ago

Hi @cstork,

This message is printed in tds_err_handler():

https://github.com/tds-fdw/tds_fdw/blob/master/src/tds_fdw.c#L4057

This could probably be fixed by checking whether dberr == 2403, and then printing it as a notice instead of an error.

cstork commented 10 months ago

Thanks @GeoffMontee,

would a pull request be considered or is the current functionality as intended?

Just asking because I found an unrelated workaround for my specific case and if you already know that you wouldn't like a change of behaviour I can save myself the effort of coming up with the PR. :-)

GeoffMontee commented 10 months ago

If you feel like contributing a PR that changes this behavior, I would definitely merge it. It is a bit odd to reject the malformed data if FreeTDS is attempting to fix it up.