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

Rebuilding secondary indexes on Android #72

Closed
ClickSimply opened this issue Jul 20, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@ClickSimply
Copy link
Owner

commented Jul 20, 2018

Issue moved from old Cordova SQLite repo, created by @phydiux.

First off, thank you for Nano-SQL!

I'm having an issue specifically related to Android and secondary indexes on tables that I define. I've recreated the issue on an example app, and I'm hoping that you can shed some light on whether it's a developer issue or a legit bug in cordova-Nano-SQLite.

In summary, when I define a table that has an index on a secondary column, on Android it runs fine the first time the app is launched and as long as there is no data in the table. When I populate that table, then kill and re-launch the app, the app throws an exception in the _NanoSQLStorage.prototype.rebuildIndexes function.

The project I put together is located here: https://github.com/phydiux/NanoSQLTest - The readme on this project shows a summary of what's happening (basically, this write-up) and a screenshot of what I'm seeing in Chrome's inspection tool (in Windows) for the app running on Android. The code should be complete enough to be able to clone, build, run and see this issue on an Android device. This is affecting another app that I'm working on that is a bit more complex, and so getting an understanding about what's going on would be very helpful for me. Thanks!

@ychsue

This comment has been minimized.

Copy link

commented Aug 3, 2018

The same problem happens in Windows UWP, too.

After looking into your code, I found that there is a promise racing when calling database/storage.ts:rebuildIndexes from line 235 of case "rebuild_idx" of database/index.ts:extend.

Because every table even _... which will be skipped by the 1st fastALL in your function rebuildIndexes will be scanned, the true tables will be still pending when all of _... tables are finished.

That's the source of the problem.
If _doCache===true, those _... tables will call this._flushIndexes() which will set this._secondaryIndexUpdates={} before those pending promises resolve and that why line 738 of rebuildIndexes will throw an error of calling 'push' of an undefined.

Well, the simplest way to avoid this problem is setting cache: false when configuring that nSQL and it works.

However, if you want to solve it, maybe you should call if(this._store._doCache) { this._store._flushIndexes(); } outside the function rebuildIndexes, i.e. in index.ts to avoid executing this condition multiple times.
For example, moving out this condition from the end of rebuildIndexes into line 237 in then of fastAll of index.ts.

It works. ^_^

@ClickSimply ClickSimply added the bug label Aug 7, 2018

@ClickSimply

This comment has been minimized.

Copy link
Owner Author

commented Aug 7, 2018

Wow Young-Chung, thanks for figuring this one out for me!

The fix is live on NPM as of 1.7.5. @phydiux, everything should work as expected now. :)

@ClickSimply ClickSimply closed this Aug 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.