Skip to content

Conversation

luissilvaos
Copy link

@luissilvaos luissilvaos commented Nov 25, 2020

Description

This PR introduces the changes that updates the SQLiteNative and the SQLGDatabaseHandle to start using the complex type for the SQL handle from the OutSystems/Android-sqlite-native-driver#1.

Context

Fixes https://outsystemsrd.atlassian.net/browse/RNMT-4515

Copy link

@cmfsotelo cmfsotelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor question before approval :)

package io.liteglue;

public class SQLiteResponse {
private int result;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, previously the errors were also returned as long
shouldn't we keep it the same way?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, SQLite result codes are integers. They were returned as long because the return type had to match either the SQLite codes and the handle value. Also, the consumer was always casting the error value to a int value as you can see here

@luissilvaos luissilvaos requested a review from oscsx November 26, 2020 09:41
@brody4hire
Copy link

Hi I just tried your OutSystems fork of cordova-sqlite-storage, which would include this update, and it does seem to resolve the issue on Pixel 5 (tested with Browser Stack).

What baffles me is that if SQLite is able to open a database, or create if it is not present, and there is no error result to be reported, how this update could really make a difference.

@luissilvaos
Copy link
Author

luissilvaos commented Mar 19, 2021

Hi I just tried your OutSystems fork of cordova-sqlite-storage, which would include this update, and it does seem to resolve the issue on Pixel 5 (tested with Browser Stack).

What baffles me is that if SQLite is able to open a database, or create if it is not present, and there is no error result to be reported, how this update could really make a difference.

@brodybits The issue is not related with the creation of the database, but the way how the connector is creating/returning the handle. Check this commit for more details: OutSystems/Android-sqlite-native-driver@6a19159

brody4hire pushed a commit to brody4hire/android-sqlite-native-ndk-connector that referenced this pull request Mar 22, 2021
Merge commit 'c4db58469edec06dbbe78caf13549b76bb61f1dc' of https://github.com/OutSystems/Android-sqlite-connector into main

include updates from this PR on the OutSystems fork:

- OutSystems/Android-sqlite-connector#1
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.

5 participants