Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
cstork opened this issue Jan 20, 2024 · 3 comments

Comments

@cstork
Copy link
Contributor

cstork commented Jan 20, 2024

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
Copy link
Collaborator

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
Copy link
Contributor Author

cstork commented Jan 21, 2024

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
Copy link
Collaborator

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.

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

No branches or pull requests

2 participants