SQL error in TDDatabase.getConflictingRevisionIDsOfDocID()

[pegli]

It seems that you aren't allowed to specify OFFSET in sqlite unless you also specify LIMIT (line 844 of TDDatabase.java):

cursor = database.rawQuery("SELECT revid FROM revs WHERE doc_id=? AND current " +
                                   "ORDER BY revid DESC OFFSET 1", args);

needs to be "ORDER BY revid DESC LIMIT -1 OFFSET 1".

[mschoch]

The grammar described on the SQLite page seems to support what you're saying:

http://www.sqlite.org/lang_select.html

I still see the same query in the iOS branch so I'm also tagging it for review by Jens.

[snej]

Yeah, it looks like the -getConflictingRevisions method was untested :( I just added a unit test for it, and sure enough, it gets a SQL error about the OFFSET keyword.

Maybe we should just delete this method, since nothing calls it?

[mschoch]

Yes, you're right its not referenced anywhere. Paul, did you uncover this reviewing the code or were you trying to use the method for something?

[pegli]

I'm working on the Android version of my TouchDB wrapper module for Titanium. The module is essentially CouchCocoa for Titanium. One of the first things I did was to port Test_Couch.m from CouchCocoa to JavaScript (test files are here if you're interested) so I can unit test my module. I found the problem because that test class contains a case called test06_History which calls CouchDocument getConflictingRevisions.

I expect that it would be useful to be able to get a list of conflicting revisions if you had to resolve any replication conflicts in TouchDB, though, so maybe it would be good to keep the method. I did change the SQL my local fork as described above and the error goes away.

[snej]

I found the problem because that test class contains a case called test06_History which calls CouchDocument getConflictingRevisions.

Let's see … ah, OK. In CouchCocoa that sends a query with a ?open_revs parameter, which in TouchDB ends up calling TDDatabase's -getAllRevisionsOfDocumentID:onlyCurrent: method. This does a very similar query, only it doesn't use a SQL OFFSET because it returns all conflicting revisions, not just the "losing" ones as -getConflictingRevisions: does.

So I think -getConflictingRevisions is unnecessary, as the aforementioned method does pretty much the same thing and is already being used.

[jchris]

We're looking into this, can you supply a quick test case? @pegli

What's the actual / expected behavior, and how can we reproduce to make sure we've fixed it?