D7492: Read correct query column indices when fetching tags

[Daniel Vrátil]

dvratil added a comment.


  TagTable has 4 columns, so  qb.addColums() on l. 77 adds 4 columns (indices 0 to 3, but we skip 3 because it's tag type ID which we don't care about in the extraction), on line 81 we add the type name column, which is 5th column (index 4), and optionally we add 6th column (remoteId) with index 5, so the current code should be correct....
  
  However it's very error-prone as if the columns in TagTable change, the code will indeed break, so please fix the way the columns are queries (see my inline comment). Then your change in the result extraction will be also correct, because we can skip querying the 4th columns (idx 3).
  
  Regarding arcanist: I think it's even harder and more annoying to use than just uploading patches via web. I don't use it, so I can't give you any advice on how to trick it into actually being helpful, rather than just plain annoying :D

INLINE COMMENTS

> tagfetchhelper.cpp:77
>      QueryBuilder qb(Tag::tableName());
>      qb.addColumns(Tag::fullColumnNames());
>  

Please replace this by

  qb.addColumn(Tag::idFullColumnName());
  qb.addColumn(Tag::gidFullColumnName());
  qb.addColumn(Tag::parentIdFullColumnName());

so that the indices don't get broken again if we add another column to the table. Then the rest of your patch will actually be correct :)

REPOSITORY
  R165 Akonadi

REVISION DETAIL
  https://phabricator.kde.org/D7492

To: dkurz, dvratil
Cc: #kde_pim, dvasin, winterz, vkrause, mlaurent, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20170823/b19c52c7/attachment.html>