ctripcorp / SQLlin

A DSL ORM library for Kotlin Multiplatform.
Apache License 2.0
221 stars 11 forks source link

Native driver does not respect isReadOnly #50

Closed nbransby closed 11 months ago

nbransby commented 11 months ago

Calling openDatabase with isReadOnly = true should use the SQLITE_OPEN_READONLY flag in NativeDatabase.openNativeDatabase

qiaoyuang commented 11 months ago

This is a known problem. This design cause I want to keep consistency with the implementation on Android. The implementation of sqllin-driver on Android base on SQLiteOpenHelper. When you call [SQLiteOpenHelper.getReadableDatabase](https://developer.android.com/reference/android/database/sqlite/SQLiteOpenHelper#getReadableDatabase()), most of time you still would get a writable database. But, obviously, the property isReadyOnly still has slightly different behavior between Android and Native platforms now, I would like to find a better way to solve this problem in the future.

nbransby commented 11 months ago

Strange the docs for SQLITE_OPEN_READWRITE state "For historical reasons, if opening in read-write mode fails due to OS-level permissions, an attempt is made to open it in read-only mode"

But that's not what we are seeing when opening a sqlite file from the main bundle (obviously read-only), we get Uncaught Kotlin exception: com.ctrip.sqllin.driver.SQLiteException: Could not open the database in read/write mode

nbransby commented 11 months ago

I assume its because SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE is used and that does not attempt the read-only mode

qiaoyuang commented 11 months ago

OK, I understand your situation. I will add the logic that use SQLITE_READ_ONLY to retry when SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE fails on native platforms.

nbransby commented 11 months ago

Yes or maybe the other way round: https://github.com/ctripcorp/SQLlin/pull/51/files

As the first call should only fail once for databases that need to be created whilst the other way round the first call will always fail for databases in readonly storage

nbransby commented 11 months ago

Updated the PR to also avoid the internal retry in sqlite ""For historical reasons, if opening in read-write mode fails due to OS-level permissions, an attempt is made to open it in read-only mode" if isReadOnly

qiaoyuang commented 11 months ago

I proposed some questions in your PR flow. We can discuss in there.

qiaoyuang commented 11 months ago

@nbransby Hi, SQLlin 1.2.1 has been published to MavenCentral. You can try it now.

nbransby commented 11 months ago

Thanks @qiaoyuang will confirm it works once tested