[ARVADOS] updated: a13c14cfc7fabe4f4da48edd57a086d2d8953a03
git at public.curoverse.com
git at public.curoverse.com
Mon Feb 8 14:29:42 EST 2016
Summary of changes:
services/api/app/models/arvados_model.rb | 4 ++
services/api/lib/load_param.rb | 26 ++++++++-
.../api/test/functional/arvados/v1/query_test.rb | 68 ++++++++++++++++++++++
3 files changed, 97 insertions(+), 1 deletion(-)
create mode 100644 services/api/test/functional/arvados/v1/query_test.rb
via a13c14cfc7fabe4f4da48edd57a086d2d8953a03 (commit)
via 78b9f61dcab9bd571952b8c9e8052b643588c52b (commit)
via 66da04e142bb80ff25d65900292bf99c7e4252f2 (commit)
from 4186924dbc103d080c47ae85bcfcd38d87314767 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit a13c14cfc7fabe4f4da48edd57a086d2d8953a03
Merge: 4186924 78b9f61
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Feb 8 14:29:03 2016 -0500
Merge branch '8289-no-extra-orders' closes #8289
commit 78b9f61dcab9bd571952b8c9e8052b643588c52b
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Feb 8 14:28:02 2016 -0500
8289: Strip redundant orders, even when provided explicitly by client.
diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index c6c8ddf..d7b9bb7 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -111,11 +111,30 @@ module LoadParam
# (e.g., [] or ['owner_uuid desc']), fall back on the default
# orders to ensure repeating the same request (possibly with
# different limit/offset) will return records in the same order.
- unless @orders.any? do |order|
- otable, ocol = order.split(' ')[0].split('.')
- otable == table_name and model_class.unique_columns.include?(ocol)
+ #
+ # Clean up the resulting list of orders such that no column
+ # uselessly appears twice (Postgres might not optimize this out
+ # for us) and no columns uselessly appear after a unique column
+ # (Postgres does not optimize this out for us; as of 9.2, "order
+ # by id, modified_at desc, uuid" is slow but "order by id" is
+ # fast).
+ orders_given_and_default = @orders + model_class.default_orders
+ order_cols_used = {}
+ @orders = []
+ orders_given_and_default.each do |order|
+ otablecol = order.split(' ')[0]
+
+ next if order_cols_used[otablecol]
+ order_cols_used[otablecol] = true
+
+ @orders << order
+
+ otable, ocol = otablecol.split('.')
+ if otable == table_name and model_class.unique_columns.include?(ocol)
+ # we already have a full ordering; subsequent entries would be
+ # superfluous
+ break
end
- @orders += model_class.default_orders
end
case params[:select]
diff --git a/services/api/test/functional/arvados/v1/query_test.rb b/services/api/test/functional/arvados/v1/query_test.rb
index 56db54c..91fe077 100644
--- a/services/api/test/functional/arvados/v1/query_test.rb
+++ b/services/api/test/functional/arvados/v1/query_test.rb
@@ -23,4 +23,46 @@ class Arvados::V1::QueryTest < ActionController::TestCase
assert_equal('logs.event_type asc, logs.modified_at desc, logs.uuid',
assigns(:objects).order_values.join(', '))
end
+
+ test 'skip fallback orders already given by client' do
+ @controller = Arvados::V1::LogsController.new
+ authorize_with :active
+ get :index, {
+ order: ['modified_at asc'],
+ controller: 'logs',
+ }
+ assert_response :success
+ assert_equal('logs.modified_at asc, logs.uuid',
+ assigns(:objects).order_values.join(', '))
+ end
+
+ test 'eliminate superfluous orders' do
+ @controller = Arvados::V1::LogsController.new
+ authorize_with :active
+ get :index, {
+ order: ['logs.modified_at asc',
+ 'modified_at desc',
+ 'event_type desc',
+ 'logs.event_type asc'],
+ controller: 'logs',
+ }
+ assert_response :success
+ assert_equal('logs.modified_at asc, logs.event_type desc, logs.uuid',
+ assigns(:objects).order_values.join(', '))
+ end
+
+ test 'eliminate orders after the first unique column' do
+ @controller = Arvados::V1::LogsController.new
+ authorize_with :active
+ get :index, {
+ order: ['event_type asc',
+ 'id asc',
+ 'uuid asc',
+ 'modified_at desc'],
+ controller: 'logs',
+ }
+ assert_response :success
+ assert_equal('logs.event_type asc, logs.id asc',
+ assigns(:objects).order_values.join(', '))
+ end
end
commit 66da04e142bb80ff25d65900292bf99c7e4252f2
Author: Tom Clegg <tom at curoverse.com>
Date: Sat Jan 23 00:23:49 2016 -0500
8289: Do not add fallback orders if client already specified an unambiguous order.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 35dd1a9..6cd40a4 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -115,6 +115,10 @@ class ArvadosModel < ActiveRecord::Base
["#{table_name}.modified_at desc", "#{table_name}.uuid"]
end
+ def self.unique_columns
+ ["id", "uuid"]
+ end
+
# If current user can manage the object, return an array of uuids of
# users and groups that have permission to write the object. The
# first two elements are always [self.owner_uuid, current user's
diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index 718aaea..c6c8ddf 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -111,7 +111,12 @@ module LoadParam
# (e.g., [] or ['owner_uuid desc']), fall back on the default
# orders to ensure repeating the same request (possibly with
# different limit/offset) will return records in the same order.
- @orders += model_class.default_orders
+ unless @orders.any? do |order|
+ otable, ocol = order.split(' ')[0].split('.')
+ otable == table_name and model_class.unique_columns.include?(ocol)
+ end
+ @orders += model_class.default_orders
+ end
case params[:select]
when Array
diff --git a/services/api/test/functional/arvados/v1/query_test.rb b/services/api/test/functional/arvados/v1/query_test.rb
new file mode 100644
index 0000000..56db54c
--- /dev/null
+++ b/services/api/test/functional/arvados/v1/query_test.rb
@@ -0,0 +1,26 @@
+require 'test_helper'
+
+class Arvados::V1::QueryTest < ActionController::TestCase
+ test 'no fallback orders when order is unambiguous' do
+ @controller = Arvados::V1::LogsController.new
+ authorize_with :active
+ get :index, {
+ order: ['id asc'],
+ controller: 'logs',
+ }
+ assert_response :success
+ assert_equal ['logs.id asc'], assigns(:objects).order_values
+ end
+
+ test 'fallback orders when order is ambiguous' do
+ @controller = Arvados::V1::LogsController.new
+ authorize_with :active
+ get :index, {
+ order: ['event_type asc'],
+ controller: 'logs',
+ }
+ assert_response :success
+ assert_equal('logs.event_type asc, logs.modified_at desc, logs.uuid',
+ assigns(:objects).order_values.join(', '))
+ end
+end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list