Merge lp:~thisfred/u1db/get-index-keys-from-index into lp:u1db
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Merged at revision: | 259 | ||||
| Proposed branch: | lp:~thisfred/u1db/get-index-keys-from-index | ||||
| Merge into: | lp:u1db | ||||
| Diff against target: | 0 lines | ||||
| To merge this branch: | bzr merge lp:~thisfred/u1db/get-index-keys-from-index | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| John A Meinel (community) | 2012-04-27 | Approve on 2012-04-27 | |
| Roberto Alsina (community) | 2012-04-25 | Approve on 2012-04-26 | |
|
Review via email:
|
|||
Commit Message
Implemented get_index_keys in all 3 backends.
Description of the Change
Implemented get_index_keys in all 3 backends.
- 259. By Eric Casteleijn on 2012-04-25
-
don't lie to me mr. docstring.
| John A Meinel (jameinel) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 4/27/2012 2:31 PM, Eric Casteleijn wrote:
> You have been requested to review the proposed merge of
> lp:~thisfred/u1db/get-index-keys-from-index into lp:u1db.
>
> For more details, see:
> https:/
>
> Implemented get_index_keys in all 3 backends.
>
> It returns a list of (key, frequency) tuples, since we can get this
> info from sqlite cheaply anyway, and it's useful for tag clouds
> etc.
>
+int
+u1db_get_
+ void *context, u1db_key_callback cb)
+{
+ int frequency;
+ int status = U1DB_OK;
+ char *key = NULL;
+ sqlite3_stmt *statement;
+ status = sqlite3_prepare_v2(
You are doing "prepare" of the statement, but you aren't finalizing it
in the 'finish' block. You'll want to add that to avoid a memory leak.
+ while (status == SQLITE_ROW) {
+ key = (char*)
+ frequency = (int)sqlite3_
+ cb(context, key, frequency);
+ status = sqlite3_
+ }
I think we should be checking the integer return value from 'cb' and
propagating it as a failure if it isn't U1DB_OK. Otherwise failures in
a cb will get silently suppressed. (I realize that isn't done
everywhere in the code, but we should.)
I'm a little hesitant to expose the count as part of the API, even if
we can get it cheaply today. Can we get it cheaply from Cassandra?
I would tend to expose less until it is found to be useful.
So some tweaks, but I think worth adding.
John
=:->
review: approve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAk+
mCQAnjEQq03UWqE
=qf9L
-----END PGP SIGNATURE-----
- 260. By Eric Casteleijn on 2012-04-27
-
added missing finalize
- 261. By Eric Casteleijn on 2012-04-27
-
deyagnification

Looks good to me