View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0005458||GNUnet||namestore service||public||2018-10-15 11:52||2019-02-28 11:17|
|Reporter||schanzen||Assigned To||Christian Grothoff|
|Product Version||Git master|
|Target Version||0.11.0||Fixed in Version||0.11.0|
|Summary||0005458: Namestore plugin API is ill-defined|
|Description||The current namestore database plugin iterator is ill-defined for the serial to exclude:|
* Iterate over the results for a particular zone in the
* datastore. Will return at most @a limit results to the iterator.
* @param cls closure (internal context for the plugin)
* @param zone private key of the zone, NULL for all zones
* @param serial serial (to exclude) in the list of matching records
* @param limit maximum number of results to return to @a iter
* @param iter function to call with the result
* @param iter_cls closure for @a iter
* @return #GNUNET_OK on success, #GNUNET_NO if there were no more results, #GNUNET_SYSERR on error
If the database backend supports (even worse: enforces) serial numbers of entries to start at 0, this means that the initial value for a serial in an iteration is undefined. The "0"th entry will never be iterated over.
Previously, this was "remedied" in the sqlite plugin by checking against "rowid >= serial", which meant that theoretically all entries were iterated over (even if sqlite supported a 0 rowid, which it doesn't), but is _also_ meant that starting an iteration from 0 and let it return one element and then continuing the iteration returned the 1st element twice because rowids start at 1!
Not to mention that this behaviour is incompatible with the API definition (the serial must be EXCLUDED)
The query is now fixed to "uid > serial", but the problem still persists for DBs that support/enforce serial numbers to start at 0. In this case, the "0th" element will never be iterated over.
In my opinion, the only solution to this problem is to redefine the plugin API so that "serial" is INCLUDED in the result.
|Tags||No tags attached.|
I don't understand why with sqlite with the old style the first element would be returned twice. The API says to exclude the given number, which admittedly implies counting from 1 (which sqlite does). Now, with the current definition (entry exclusive), we ask for > 0 and should get 1, then we should ask for 1 and get 2. So why do you say we get 1 twice!?
Anyway, I'm not against changing the definition as you suggest, I just don't fully appreciate why there is this issue.
Ok. I misunderstood the issue myself. You are right.
However, we do not ask for >0 in the SQL query. That is also why we violated the API _again_ by returning seq+1 to the callback (where we should return seq but that would have resulted in a loop).
I think what I observed from the zonemaster callbacks (and the namestore service) and what I saw in the code lead me on a wrong path.
BUT, that does not mean it doesn't happen! If you revert my edits and iterate over all zones, you get duplicates.
Since it works now, I am too lazy to check why.
I modified the plugin to adhere to the API now, but the default value/zero serial is still undefined.
|7285ae216..443454a62 now clarifies that a serial number of 0 must never be returned by the namestore plugins. I've also gone over them to make sure they abide by the API description.|
|2018-10-15 11:52||schanzen||New Issue|
|2018-10-15 11:52||schanzen||Status||new => assigned|
|2018-10-15 11:52||schanzen||Assigned To||=> Christian Grothoff|
|2018-10-15 15:09||Christian Grothoff||Note Added: 0013270|
|2018-10-15 16:31||schanzen||Note Added: 0013271|
|2019-02-13 23:19||Christian Grothoff||Note Added: 0013720|
|2019-02-13 23:19||Christian Grothoff||Status||assigned => resolved|
|2019-02-13 23:19||Christian Grothoff||Resolution||open => fixed|
|2019-02-13 23:19||Christian Grothoff||Fixed in Version||=> 0.11.0|
|2019-02-13 23:19||Christian Grothoff||Product Version||=> Git master|
|2019-02-13 23:19||Christian Grothoff||Target Version||=> 0.11.0|
|2019-02-28 11:17||Christian Grothoff||Status||resolved => closed|