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

Nominal support for wide char params binding #68

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

staticlibs
Copy link
Contributor

@staticlibs staticlibs commented Feb 23, 2025

Edit: up to date description:

This change adds only a nominal support for passing query parameters
as SQL_WVARCHAR, it uses existing logic that is already there for
SQL_WCHAR parameters.

It allows to use parameters from PyODBC, but in current impl only ASCII
data can be passed in these parameters.

Testing: none, test suite needs to be extended to test unicode through
the driver manager (related issue - #69).

Original description (encoding conversion removed in PR update):

Before this change the binding for SQL_WCHAR parameters was declared in ParameterDescriptor::SetValue but was not actually implemented (no encoding conversion).

This change implements support for getting incoming SQL_WCHAR/SQL_WVARCHAR parameters as normal UTF-8 values. Incoming UCS-2 (on Windows) or UTF-32 (on other platforms) parameter data is converted to UTF-8 using C++ stdlib.

Testing: added a test that inserts non-ASCII data as literal and then uses wide char parameter binding to match this data in SELECT.

Fixes: #49

@staticlibs
Copy link
Contributor Author

staticlibs commented Feb 23, 2025

Apaprently this does not work with Windows ODBC driver manager that uses Unicode-to-ANSI mapping with DuckDB driver. It seems that double conversion is happening, moving this PR to Draft, will follow up on it.

This change adds only a nominal support for passing query parameters
as `SQL_WVARCHAR`, it uses existing logic that is already there for
`SQL_WCHAR` parameters.

It allows to use parameters from PyODBC, but in current impl only ASCII
data can be passed in these parameters.

Testing: none, test suite needs to be extended to test unicode through
the driver manager.

Fixes: duckdb#49
@staticlibs staticlibs force-pushed the wide_char_params_binding branch from 5a1ae8c to 2a896d7 Compare February 24, 2025 10:19
@staticlibs staticlibs marked this pull request as ready for review February 24, 2025 10:19
@staticlibs staticlibs changed the title Support wide char params binding Nominal support for wide char params binding Feb 24, 2025
@staticlibs
Copy link
Contributor Author

Updated the PR leaving only the nominal support for SQL_WVARCHAR to not throw error on PyODBC parameters binding. Filed #69 for proper unicode support.

Encoding conversion is removed, the C++ stdlibs seem to be inadequate for this task because it throws on failure. But the correct way should be to do input conversion as lenient as possible (with optional strict mode) to not break users' ETL pipelines because of invalid UTF-8 sequence in one field.

Also testing with linking directly to driver does not seem to be useful, some wider testing setup is needed that goes though the driver manager.

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

Successfully merging this pull request may close these issues.

Cannot parameterize SQL with strings using pyodbc
1 participant