Arvados

This is implemented in arvados/services/api/lib

          when 'is_a'
            operand = [operand] unless operand.is_a? Array
            cond = []
            operand.each do |op|
              cl = ArvadosModel::kind_class op
              if cl
                cond << "#{ar_table_name}.#{attr} like ?" 
                param_out << cl.uuid_like_pattern
              else
                cond << "1=0" 
              end
            end
            cond_out << cond.join(' OR ')

Instead of using 'like' this should use a substring match:

            operand.each do |op|
              cl = ArvadosModel::kind_class op
              if cl
                cond << "substring(#{ar_table_name}.#{attr}, 6, 5) = ?" 
                param_out << cl.uuid_prefix
              else
                cond << "1=0" 
              end
            end

If 'attr' is 'uuid' I think you can even short circuit the check with "model_class.uuid_prefix == cl.uuid_prefix" (we can assume every value in the uuid column has the same type.)

To ensure the substring match is efficient, we may need to create an expression index:

https://www.postgresql.org/docs/9.4/static/indexes-expressional.html

Updates at cd7a746df - branch 13006-api-is_a-filter
Test run: https://ci.curoverse.com/job/developer-run-tests/1009/

• Followed Peter's suggestion by changing the query to use substring(), avoiding the check when attr is "uuid" and prefix_uuid matches.
• Added expression index on links.[tail|head]_uuid
• Added test

I also could update the group-sync tool to use the is_a operator again, for enhanced performance.

Lucas Di Pentima wrote:

Updates at cd7a746df - branch 13006-api-is_a-filter
Test run: https://ci.curoverse.com/job/developer-run-tests/1009/

• Followed Peter's suggestion by changing the query to use substring(), avoiding the check when attr is "uuid" and prefix_uuid matches.

If attr == 'uuid' but model_class.uuid_prefix != cl.uuid_prefix, should it fail unconditionally?

• Added expression index on links.[tail|head]_uuid
• Added test

I also could update the group-sync tool to use the is_a operator again, for enhanced performance.

How about in a separate branch.

Peter Amstutz wrote:

If attr == 'uuid' but model_class.uuid_prefix != cl.uuid_prefix, should it fail unconditionally?

I believe so, fixed.

Update at 96165e8fc
Test run: https://ci.curoverse.com/job/developer-run-tests/1011/

Peter Amstutz wrote:

I also could update the group-sync tool to use the is_a operator again, for enhanced performance.

How about in a separate branch.

Done: 84dcca64f - branch 13006-sync-groups-is_a-filter
Test run: https://ci.curoverse.com/job/developer-run-tests/1012/ (not yet started atm)

Lucas Di Pentima wrote:

Peter Amstutz wrote:

I also could update the group-sync tool to use the is_a operator again, for enhanced performance.

How about in a separate branch.

Done: 84dcca64f - branch 13006-sync-groups-is_a-filter
Test run: https://ci.curoverse.com/job/developer-run-tests/1012/ (not yet started atm)

Is there a 'sync-groups' test that fails without the '13006-api-is_a-filter' fix but passes with it?

Yes. Although I didn't tested it now, but it was the means to detect this is_a bug in the API server. The test is TestMembershipRemoval, if you want I can rebase this branch to master and then merge the other 13006 branch to prove that the test passes.

Lucas Di Pentima wrote:

Yes. Although I didn't tested it now, but it was the means to detect this is_a bug in the API server. The test is TestMembershipRemoval, if you want I can rebase this branch to master and then merge the other 13006 branch to prove that the test passes.

Yes, I think that would be a good idea. Go ahead and merge both branches if it goes as expected (test fails without fix → test passes with fix)

Ok, so I rebased 13006-sync-groups-is_a-filter (0c21d2bd2d0fd351a4e546002a249b0b748061eb) to master. Ran the sync-groups tests locally and got the failure:

FAIL: sync-groups_test.go:267: TestSuite.TestMembershipRemoval

sync-groups_test.go:308:
    c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID), Equals, false)
... obtained bool = true
... expected bool = false

Then, merged 13006-api-is_a-filter into 13006-sync-groups-is_a-filter (cbf93e8d897448dbd52369afe89fef2392140ff1). Tests now pass.