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

Support boolean type in params array #61

Merged
merged 1 commit into from
Jun 24, 2016
Merged

Conversation

anativ
Copy link
Contributor

@anativ anativ commented Jun 21, 2016

When trying to send "boolean" as param the query hangs on iPhone
This updates allows to send booleans (true=1 / false=0)

@andpor
Copy link
Owner

andpor commented Jun 21, 2016

Can you create an issue for this so I can link it up ?

@anativ
Copy link
Contributor Author

anativ commented Jun 21, 2016

There is issue #45 if you want I can open a new one

@anativ
Copy link
Contributor Author

anativ commented Jun 22, 2016

I opened a better issue for that #63

@brodybits
Copy link
Contributor

This change could break Web SQL compatibility. When I tested with both my Cordova sqlite plugin and Web SQL I discovered that the Boolean values false and true are inserted as the string "false" and "true" values. My sqlite plugin uses the toString function to get the string value of non-primitive objects, and I suspect that Web SQL follows a similar strategy.

Cordova sqlite JavaScript code: https://github.com/litehelpers/Cordova-sqlite-storage/blob/storage-master/www/SQLitePlugin.js#L354-L358

test: https://github.com/litehelpers/Cordova-sqlite-storage/blob/storage-master/spec/www/spec/db-tx-value-bindings-test.js#L254-L284

@anativ
Copy link
Contributor Author

anativ commented Jun 22, 2016

@brodybits I understand your concern but when sending a boolean as params on IOS device or Simulator (when not using debugger) the "executeSql" never returns (no success / error). thats not a valid option.
Anyway "true" / "false" as strings is a bug even if Web SQL acts like that.
As I see it there are two options

  1. convert boolean to number (true=>1/false=>0) - this commit
  2. return error

Thanks

@brodybits
Copy link
Contributor

I think it would be worse to return an error since it could really break code that was working for Web SQL. I think you have a good point to convert the boolean to a number since SQLite takes the same approach: https://www.sqlite.org/datatype3.html#section_2_1

@andpor
Copy link
Owner

andpor commented Jun 22, 2016

This plugin will not divert in the fine SQL handling details from Cordova-sqlite-storage. It seems that currently the code is designed to handle boolean by converting the value to string using toString method but I am tempted to make an adjustment to convert it to 0 and 1.

https://github.com/andpor/react-native-sqlite-storage/blob/master/lib/sqlite.core.js#L421-L429

I am curious about the fact that executeSql never returns and provides no feedback. Can you send the sample query you are having problem with that would demonstrate the issue?

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.

None yet

3 participants