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

Add the Connection property charset to support from URL #24

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

Conversation

rtyuy
Copy link

@rtyuy rtyuy commented Mar 5, 2025

Add the Connection property charset to support setting the encoding from the URL. For example, the encoding of early MDB files was not fixed and depended on the encoding used when they were created. When reading older MDB files and the encoding does not match the system's default encoding, it can cause garbled text issues. Therefore, adding an encoding property makes it convenient to set the encoding, such as jdbc:ucanaccess://d:/test.mdb;CHARSET=gbk.

…rom the URL. For example, the encoding of early MDB files was not fixed and depended on the encoding used when they were created. When reading older MDB files and the encoding does not match the system's default encoding, it can cause garbled text issues. Therefore, adding an encoding property makes it convenient to set the encoding, such as jdbc:ucanaccess://d:/test.mdb;CHARSET=GBK
Copy link

sonarqubecloud bot commented Mar 5, 2025

@spannm
Copy link
Owner

spannm commented Mar 7, 2025

Hello @rtyuy,

It's a good suggestion to add this property.

I've gone over your pr, looking good but I would like some minor adjustments:

  • Overload IJackcessOpenerInterface#open and provide default implementation (keeping original signature as-is for backward compatibility)
  • Handle UnsupportedCharsetException gracefully in Charset#forName(String) with a meaningful exception message

My additions here: https://github.com/spannm/ucanaccess/tree/pr-24

Do you think you could add a unit test with a database file that uses an uncommon charset?
The db file should be as small as possible in order not to bloat the repo. Look at any JUnit test that extends UcanaccessBaseFileTest as an example. The test database name is derived from the unit test name e.g. MetaDataParameterizedTest ==> metaDataParameterized.accb.

Cheers
Markus

@rtyuy
Copy link
Author

rtyuy commented Mar 13, 2025

ok, I will put a unit test later

@spannm
Copy link
Owner

spannm commented Mar 25, 2025

Hi @rtyuy,

I've added the feature with minor modifications.

Still, we should add a unit test. Please upload a small mdb with GBK character encoding (the smaller the better in order not to bloat the repo) to this thread or provide a complete unit test.

cheers
Markus

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.

2 participants