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
only-cliches opened this issue Jul 20, 2018 · 2 comments
Closed

Rebuilding secondary indexes on Android #72

only-cliches opened this issue Jul 20, 2018 · 2 comments
Labels

Comments

@only-cliches
Copy link
Owner

@only-cliches only-cliches 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
Copy link

@ychsue ychsue 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. ^_^

@only-cliches
Copy link
Owner Author

@only-cliches only-cliches 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. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants