https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422015-06-04T05:12:16ZArvadosArvados - Bug #6240: [API] "order" params should be rejected or used (not silently ignored) even when they reference columns missing from the "select" paramhttps://dev.arvados.org/issues/6240?journal_id=255222015-06-04T05:12:16ZTom Cleggtom@curii.com
<ul><li><strong>Project</strong> changed from <i>35</i> to <i>Arvados</i></li><li><strong>Description</strong> updated (<a title="View differences" href="/journals/25522/diff?detail_id=24935">diff</a>)</li><li><strong>Category</strong> set to <i>API</i></li><li><strong>Target version</strong> set to <i>Bug Triage</i></li></ul><p>Noticed during <a class="issue tracker-1 status-3 priority-4 priority-default closed parent" title="Bug: [API] collections are being returned too slowly + NoMemoryError (Resolved)" href="https://dev.arvados.org/issues/6074">#6074</a>.</p> Arvados - Bug #6240: [API] "order" params should be rejected or used (not silently ignored) even when they reference columns missing from the "select" paramhttps://dev.arvados.org/issues/6240?journal_id=257432015-06-10T13:54:30ZTom Cleggtom@curii.com
<ul></ul><p>The easy fix (reject instead of ignoring) looks something like this, in <a class="source" href="https://dev.arvados.org/projects/arvados/repository/arvados/entry/services/api/lib/load_param.rb">source:services/api/lib/load_param.rb</a></p>
<pre><code class="diff syntaxhl"> if @select
# Any ordering columns must be selected when doing select,
# otherwise it is an SQL error, so filter out invaliding orderings.
<span class="gd">- @orders.select! { |o|
</span><span class="gi">+ @orders.each { |o|
</span> # match select column against order array entry
<span class="gd">- @select.select { |s| /^#{table_name}.#{s}( (asc|desc))?$/.match o }.any?
</span><span class="gi">+ if not @select.select { |s| /^#{table_name}.#{s}( (asc|desc))?$/.match o }.any?
+ raise ArgumentError.new("Cannot order on '#{s}' when column excluded by select param")
+ end
</span> }
end
</code></pre>
<p>IMO it would be vastly preferable to add the given column to the list of columns to select (without adding it to the list of attributes to return to the client). That <em>might</em> involve changing more than 2 lines of code, though.</p> Arvados - Bug #6240: [API] "order" params should be rejected or used (not silently ignored) even when they reference columns missing from the "select" paramhttps://dev.arvados.org/issues/6240?journal_id=286082015-08-11T14:09:21ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Target version</strong> changed from <i>Bug Triage</i> to <i>Arvados Future Sprints</i></li></ul> Arvados - Bug #6240: [API] "order" params should be rejected or used (not silently ignored) even when they reference columns missing from the "select" paramhttps://dev.arvados.org/issues/6240?journal_id=353832016-02-17T19:32:26ZJoshua Randalljr17@sanger.ac.uk
<ul></ul><p>I just got caught by this. Definitely was not expecting the selected columns to have any bearing on the requested order. Not really understanding why this is not a simple thing to do... must be some difficulties with the Ruby ORM?</p> Arvados - Bug #6240: [API] "order" params should be rejected or used (not silently ignored) even when they reference columns missing from the "select" paramhttps://dev.arvados.org/issues/6240?journal_id=949832021-07-07T18:29:19ZWard Vandewegeward@curii.com
<ul><li><strong>Target version</strong> deleted (<del><i>Arvados Future Sprints</i></del>)</li></ul> Arvados - Bug #6240: [API] "order" params should be rejected or used (not silently ignored) even when they reference columns missing from the "select" paramhttps://dev.arvados.org/issues/6240?journal_id=1122312023-02-14T22:24:36ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Release</strong> set to <i>60</i></li></ul> Arvados - Bug #6240: [API] "order" params should be rejected or used (not silently ignored) even when they reference columns missing from the "select" paramhttps://dev.arvados.org/issues/6240?journal_id=1235892024-03-01T21:15:53ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Target version</strong> set to <i>Future</i></li></ul>