483318 - mozIStorageStatementParams doesn't let you bind an array

Status: RESOLVED FIXED

(Core :: SQLite and Embedded Database Bindings, defect, P3)

Description

[Shawn Wilsher :sdwilsh]

And we really should be able to bind an array of ints, which is how we return binary blobs anyway.

Comment 1

[alta88]

bindBlobParameter() works fine, but i notice it's deprecated..

Comment 2

[Shawn Wilsher :sdwilsh]

(In reply to comment #1)
> bindBlobParameter() works fine, but i notice it's deprecated..
bindBlobByName or bindBlobByIndex have replaced it.

Comment 3

[Marco Bonardo [:mak]]

We could use https://www.sqlite.org/carray.html but we should be careful, the provided query should never allow an user provided argument, we should only support it through Storage API. Note: I'm not sure which part of the carray syntax, if any, can be bound, I'm assuming its arguments can, so the idea currently is:
parse queries for "carray" and throw  if the any of its 2 arguments is not bound.
Sqlite.jsm may also allow something like "IN :myarray" or "IN ?" and just do a string replacement internally, while cpp API may only support a straight carray(?,?) or carray(:myarray, :mylen).

Comment 4

[Marco Bonardo [:mak]]

Attachment: WIP: Bug 483318 - Vendor sqlite3 carray extension

Comment 5

[Marco Bonardo [:mak]]

Attachment: WIP: Bug 483318 - Bind array in mozStorage

Depends on D221264

Comment 6

[Marco Bonardo [:mak]]

Attachment: Bug 483318 - Vendor SQLite carray extension and load it automatically. r=asuth,lina

Comment 7

[Marco Bonardo [:mak]]

Attachment: Bug 483318 - Implement binding arrays to mozIStorageStatementParams. r=asuth,lina

Use carray`` extension to bind array of numerics and strings, so we can avoid large and slow IN clauses, and cache prepared statements having a variable number of parameters. The extension is statically loaded and available in every connection. Consumers are encouraged to use the explicit bindArrayXXXmethods, as the genericbindByIndexandbindByNamemethods are too lenient, especially from Javascript. Notecarray`` only supports UTF8 encoded strings, the API will convert
the encoding when UTF16 is passed in.
These new variants are not exposed to Rust yet, as the existing comment
suggests they were intended for primitive types. It could be done in the
future, if necessary.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/507b979e3c93
Vendor SQLite carray extension and load it automatically. r=asuth,lina,glandium
https://hg.mozilla.org/integration/autoland/rev/d23ddf67f8a4
Implement binding arrays to mozIStorageStatementParams. r=asuth,lina,Standard8

Comment 9

[agoloman]

Backed out for causing multiple failures.

• Backout link

• Push with failures -GTest failures

• Failure Log
  ————————————————————————————————————

• Push with failures - build bustages

• Failure Log
  ————————————————————————————————————

• Push with failures - another gtest failure

• Failure Log
  ————————————————————————————————————

• Push with failures - valgrind bustages

• Failure Log

Comment 10

[Marco Bonardo [:mak]]

The gtest failure is funny, it is actually caused by a previous test that is calling sqlite3_shutdown() and then it tries to reinitialize the library, but it doesn't use the same options that were originally used to initialize it.
So the solution is to expose an Init() method from AutoSQLiteLifetime and use that to reinitialize.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c03fc306baf
Vendor SQLite carray extension and load it automatically. r=asuth,lina,glandium
https://hg.mozilla.org/integration/autoland/rev/7884cf6b09a9
Implement binding arrays to mozIStorageStatementParams. r=asuth,lina,Standard8

Comment 12

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/8c03fc306baf
https://hg.mozilla.org/mozilla-central/rev/7884cf6b09a9