requery / sqlite-android

Android SQLite support library
Apache License 2.0
1.06k stars 139 forks source link

Locale Issues: Requery Behaving Differently Than Framework SQLite #55

Open commonsguy opened 6 years ago

commonsguy commented 6 years ago

While testing CWAC-SafeRoom, I converted a bunch of existing tests for AOSP and SQLCipher for Android into a test suite that exercises the support database API.

When testing Requery, most things work. However, I have two failing tests, ones that work with the official support database implementation (FrameworkSQLiteOpenHelperFactory and kin).

The attached project is a subset of the full test suite that reproduces the two test failures:

RequeryLocale.zip

The icu() test method fails the official implementation on Android 4.4 and older, so I have that test rigged to only fail "for realz" on Android 5.0+. There, the official implementation works, but Requery doesn't. The testLocaleenUS() fails going all the way back to Android 4.1, but the official implementation works.

Both failures are tied to locale:

I didn't write either test -- these are both from the AOSP, IIRC. But, the framework SQLite implementation is effectively the reference implementation with respect to third-party support database implementations like yours and mine. I wasn't sure if you were aware of this difference in behavior or not, or whether you consider it "different, but working as intended" or not.

(and FWIW, CWAC-SafeRoom fails on these too, so I'll be working with Zetetic to figure out what this means)

Let me know if you need additional information!

npurushe commented 6 years ago

Yes in order for those tests to pass the SQlite ICU extension must be loaded. The Android OS SQLite is compiled with this extension enabled since it can easily link into the native libicu that is already present. Unfortunately an android library like this one can't do the same, libicu is not available through the ndk and building it as a standalone library can be complicated. However it something that would be nice to support through an optional extension see #30.

commonsguy commented 6 years ago

OK, that makes sense. Do you want to consider this a duplicate of #30?

npurushe commented 6 years ago

Looks like this will become easier with https://github.com/android-ndk/ndk/issues/548 in NDK r17. Will leave this open as a enhancement thanks!