<table><tr><td style="">dvratil added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D7492" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>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....</p>

<p>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).</p>

<p>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</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7492#inline-30533" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">tagfetchhelper.cpp:77</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; ">    <span class="n">QueryBuilder</span> <span class="n">qb</span><span class="p">(</span><span class="n">Tag</span><span style="color: #aa2211">::</span><span class="n">tableName</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">    <span class="n">qb</span><span class="p">.</span><span class="n">addColumns</span><span class="p">(</span><span class="n">Tag</span><span style="color: #aa2211">::</span><span class="n">fullColumnNames</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please replace this by</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">qb.addColumn(Tag::idFullColumnName());
qb.addColumn(Tag::gidFullColumnName());
qb.addColumn(Tag::parentIdFullColumnName());</pre></div>

<p style="padding: 0; margin: 8px;">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 :)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R165 Akonadi </div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D7492" rel="noreferrer">https://phabricator.kde.org/D7492</a></div></div><br /><div><strong>To: </strong>dkurz, dvratil<br /><strong>Cc: </strong>KDE PIM, dvasin, winterz, vkrause, mlaurent, knauss, dvratil<br /></div>