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

Fix parsing conn string with a trailing semicolon #53

Merged
merged 2 commits into from
Feb 2, 2025

Conversation

staticlibs
Copy link
Contributor

@staticlibs staticlibs commented Jan 26, 2025

Hi,

This PR fixes the connection string parsing for cases, when such string ends with a semicolon.

When connection string includes a trailing semicolon, existing impl in Connect::ParseInputStr was doing an early exit without setting DSN value to dbc->dsn. Because of this DSN was not used and default :memory: instance was created.

The problem is not OS specific, but was happening on Windows all the time because Windows ODBC Driver Manager is appending semicolon to the passed connection string when calling SQLDriverConnect.

Testing: test_connect_odbc was extended to cover the trailing semicolon and also parameters overriding in connection string.

Fixes: #29, #48

Edit: typo in a commit mesage: %s/comma/semicolon/g.

@staticlibs staticlibs changed the title Fix parsing conn string with a trailing comma Fix parsing conn string with a trailing semicolon Jan 26, 2025
@staticlibs
Copy link
Contributor Author

Hi @Mytherin, @guenp , I wonder if you can review this PR? It is basically a continuation of #46 that was recently reviewed/merged by you. Thanks!

@Mytherin
Copy link
Contributor

Mytherin commented Feb 1, 2025

Thanks for the PR! Looks good - could you just merge with main so the CI can re-run? I've patched the issue with the CI in #55

When connection string includes a trailing comma, existing impl in
Connect::ParseInputStr was doing an early exit without setting DSN
value to dbc->dsn. Because of this DSN was not used and default
:memory: instance was created.

The problem is not OS specific, but was happening on Windows all the
time because Windows ODBC Driver Manager is appending comma to the
passed connection string when calling SQLDriverConnect.

test_connect_odbc was extended to cover the trailing comma and also
parameters overriding in connection string.

Fixes: duckdb#29, duckdb#48
@staticlibs
Copy link
Contributor Author

Thanks! I've rebased the branch onto main.

@Mytherin Mytherin merged commit d6458db into duckdb:main Feb 2, 2025
6 checks passed
@Mytherin
Copy link
Contributor

Mytherin commented Feb 2, 2025

Thanks!

@staticlibs staticlibs deleted the dsn_trailing_comma branch February 13, 2025 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants