{"action":"created","comment":{"url":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/comments/59075142","id":59075142,"diff_hunk":"@@ -13,17 +13,32 @@ def self.transitive_closure?(ancestor, descendant)\n       end\n \n       def self.descendants_of(element_or_scope)\n-        recursive_query = join_recursive do |query|\n-          query.start_with(parent_id: element_or_scope).connect_by(child_id: :parent_id).nocycle\n+        #FIXME: Preloading with to_a seems to be necessary because putting complex sql in start_with can\n+        # lead to degenerate performance (noticed in ancestors_of call in accessible_objects)\n+        # Ideally, fix the SQL so it's both a single call and performant\n+        element_or_scope = [*element_or_scope]\n+        case element_or_scope.size\n+        when 0\n+          PolicyElement.none\n+        when 1\n+          PolicyElement.where('\"policy_elements\".\"id\" IN (SELECT assignments__recursive.child_id FROM (WITH RECURSIVE \"assignments__recursive\" AS ( SELECT \"assignments\".\"id\", \"assignments\".\"child_id\", \"assignments\".\"parent_id\" FROM \"assignments\" WHERE \"assignments\".\"parent_id\" = ? UNION ALL SELECT \"assignments\".\"id\", \"assignments\".\"child_id\", \"assignments\".\"parent_id\" FROM \"assignments\" INNER JOIN \"assignments__recursive\" ON \"assignments__recursive\".\"child_id\" = \"assignments\".\"parent_id\" ) SELECT \"assignments__recursive\".* FROM \"assignments__recursive\") AS \"assignments__recursive\")', element_or_scope.first.id)","path":"lib/policy_machine_storage_adapters/active_record/postgresql.rb","position":14,"original_position":14,"commit_id":"ac2cd1d8c7ba8ed82af966b469c3594952de2241","original_commit_id":"ac2cd1d8c7ba8ed82af966b469c3594952de2241","user":{"login":"clabrunda","id":2256233,"avatar_url":"https://avatars.githubusercontent.com/u/2256233?v=3","gravatar_id":"","url":"https://api.github.com/users/clabrunda","html_url":"https://github.com/clabrunda","followers_url":"https://api.github.com/users/clabrunda/followers","following_url":"https://api.github.com/users/clabrunda/following{/other_user}","gists_url":"https://api.github.com/users/clabrunda/gists{/gist_id}","starred_url":"https://api.github.com/users/clabrunda/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/clabrunda/subscriptions","organizations_url":"https://api.github.com/users/clabrunda/orgs","repos_url":"https://api.github.com/users/clabrunda/repos","events_url":"https://api.github.com/users/clabrunda/events{/privacy}","received_events_url":"https://api.github.com/users/clabrunda/received_events","type":"User","site_admin":false},"body":"Would it be easier to read these if they were broken up into multiple lines?\r\nDoes this look any better:\r\n```\r\n          PolicyElement.where(\r\n            '\"policy_elements\".\"id\" IN (SELECT assignments__recursive.child_id '\\\r\n            'FROM (WITH RECURSIVE \"assignments__recursive\" AS '\\\r\n            '( SELECT \"assignments\".\"id\", \"assignments\".\"child_id\", \"assignments\".\"parent_id\" FROM \"assignments\" '\\\r\n            'WHERE \"assignments\".\"parent_id\" = ? '\\\r\n            'UNION ALL SELECT \"assignments\".\"id\", \"assignments\".\"child_id\", \"assignments\".\"parent_id\" FROM \"assignments\" '\\\r\n            'INNER JOIN \"assignments__recursive\" ON \"assignments__recursive\".\"child_id\" = \"assignments\".\"parent_id\" ) '\\\r\n            'SELECT \"assignments__recursive\".* FROM \"assignments__recursive\")'\\\r\n            'AS \"assignments__recursive\")', element_or_scope.first.id)'\r\n```","created_at":"2016-04-08T19:11:22Z","updated_at":"2016-04-08T19:11:22Z","html_url":"https://github.com/mdsol/the_policy_machine/pull/54#discussion_r59075142","pull_request_url":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/54","_links":{"self":{"href":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/comments/59075142"},"html":{"href":"https://github.com/mdsol/the_policy_machine/pull/54#discussion_r59075142"},"pull_request":{"href":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/54"}}},"pull_request":{"url":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/54","id":65542309,"html_url":"https://github.com/mdsol/the_policy_machine/pull/54","diff_url":"https://github.com/mdsol/the_policy_machine/pull/54.diff","patch_url":"https://github.com/mdsol/the_policy_machine/pull/54.patch","issue_url":"https://api.github.com/repos/mdsol/the_policy_machine/issues/54","number":54,"state":"open","locked":false,"title":"Break recursive queries into multiple calls to avoid confusing the optimizer","user":{"login":"HonoreDB","id":1795573,"avatar_url":"https://avatars.githubusercontent.com/u/1795573?v=3","gravatar_id":"","url":"https://api.github.com/users/HonoreDB","html_url":"https://github.com/HonoreDB","followers_url":"https://api.github.com/users/HonoreDB/followers","following_url":"https://api.github.com/users/HonoreDB/following{/other_user}","gists_url":"https://api.github.com/users/HonoreDB/gists{/gist_id}","starred_url":"https://api.github.com/users/HonoreDB/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/HonoreDB/subscriptions","organizations_url":"https://api.github.com/users/HonoreDB/orgs","repos_url":"https://api.github.com/users/HonoreDB/repos","events_url":"https://api.github.com/users/HonoreDB/events{/privacy}","received_events_url":"https://api.github.com/users/HonoreDB/received_events","type":"User","site_admin":false},"body":"Not quite ready to call this done, there's lots more savings to get by removing the unnecessary join of Assignment onto the result, and also I don't understand how or why any of this works, but it reduces a 30-second call to a second and a half on sandbox so I'm getting it out there for posterity. The SQL this generates is \r\n```sql\r\nSELECT \"policy_elements\".* FROM \"policy_elements\" WHERE \"policy_elements\".\"id\" IN (SELECT assignments__recursive.parent_id FROM \"assignments\" INNER JOIN (WITH RECURSIVE \"assignments__recursive\" AS ( SELECT \"assignments\".\"id\", \"assignments\".\"parent_id\", \"assignments\".\"child_id\" FROM \"assignments\" WHERE \"assignments\".\"child_id\" IN (1876421, 1876537, 1879376, 1878545, 1876592, 1876647, 1878453, 1878913, 1879042, 1879211, 1879432, 1879318, 1879317, 1876423, 1879096, 1876648, 1878914, 1879377, 1879097, 1878454, 1876593, 1879043, 1879212, 1879431, 1878544, 1876536) UNION ALL SELECT \"assignments\".\"id\", \"assignments\".\"parent_id\", \"assignments\".\"child_id\" FROM \"assignments\" INNER JOIN \"assignments__recursive\" ON \"assignments__recursive\".\"parent_id\" = \"assignments\".\"child_id\" ) SELECT \"assignments__recursive\".* FROM \"assignments__recursive\") AS \"assignments__recursive\" ON \"assignments\".\"id\" = \"assignments__recursive\".\"id\") AND \"policy_elements\".\"type\" = 'PolicyMachineStorageAdapter::ActiveRecord::Object'\r\n```\r\n\r\nThe SQL I wish it generated because it's much faster is\r\n\r\n```SQL\r\nSELECT \"policy_elements\".* FROM \"policy_elements\" WHERE \"policy_elements\".\"id\" IN (SELECT assignments__recursive.parent_id FROM (WITH RECURSIVE \"assignments__recursive\" AS ( SELECT \"assignments\".\"id\", \"assignments\".\"parent_id\", \"assignments\".\"child_id\" FROM \"assignments\" WHERE \"assignments\".\"child_id\" IN (1876421, 1876537, 1879376, 1878545, 1876592, 1876647, 1878453, 1878913, 1879042, 1879211, 1879432, 1879318, 1879317, 1876423, 1879096, 1876648, 1878914, 1879377, 1879097, 1878454, 1876593, 1879043, 1879212, 1879431, 1878544, 1876536) UNION ALL SELECT \"assignments\".\"id\", \"assignments\".\"parent_id\", \"assignments\".\"child_id\" FROM \"assignments\" INNER JOIN \"assignments__recursive\" ON \"assignments__recursive\".\"parent_id\" = \"assignments\".\"child_id\" ) SELECT \"assignments__recursive\".* FROM \"assignments__recursive\") AS \"assignments__recursive\") AND \"policy_elements\".\"type\" = 'PolicyMachineStorageAdapter::ActiveRecord::Object';\r\n```\r\n\r\nwhich should be equivalent, but it'll take more work to verify it actually is and to persuade ActiveRecord to generate it.","created_at":"2016-04-06T21:50:30Z","updated_at":"2016-04-08T19:11:22Z","closed_at":null,"merged_at":null,"merge_commit_sha":"3f74d4baa616ecbbd5b159b0f98fb25a27c42e4d","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/54/commits","review_comments_url":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/54/comments","review_comment_url":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mdsol/the_policy_machine/issues/54/comments","statuses_url":"https://api.github.com/repos/mdsol/the_policy_machine/statuses/ac2cd1d8c7ba8ed82af966b469c3594952de2241","head":{"label":"mdsol:fix/ancestors_of_performance","ref":"fix/ancestors_of_performance","sha":"ac2cd1d8c7ba8ed82af966b469c3594952de2241","user":{"login":"mdsol","id":171103,"avatar_url":"https://avatars.githubusercontent.com/u/171103?v=3","gravatar_id":"","url":"https://api.github.com/users/mdsol","html_url":"https://github.com/mdsol","followers_url":"https://api.github.com/users/mdsol/followers","following_url":"https://api.github.com/users/mdsol/following{/other_user}","gists_url":"https://api.github.com/users/mdsol/gists{/gist_id}","starred_url":"https://api.github.com/users/mdsol/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mdsol/subscriptions","organizations_url":"https://api.github.com/users/mdsol/orgs","repos_url":"https://api.github.com/users/mdsol/repos","events_url":"https://api.github.com/users/mdsol/events{/privacy}","received_events_url":"https://api.github.com/users/mdsol/received_events","type":"Organization","site_admin":false},"repo":{"id":13685902,"name":"the_policy_machine","full_name":"mdsol/the_policy_machine","owner":{"login":"mdsol","id":171103,"avatar_url":"https://avatars.githubusercontent.com/u/171103?v=3","gravatar_id":"","url":"https://api.github.com/users/mdsol","html_url":"https://github.com/mdsol","followers_url":"https://api.github.com/users/mdsol/followers","following_url":"https://api.github.com/users/mdsol/following{/other_user}","gists_url":"https://api.github.com/users/mdsol/gists{/gist_id}","starred_url":"https://api.github.com/users/mdsol/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mdsol/subscriptions","organizations_url":"https://api.github.com/users/mdsol/orgs","repos_url":"https://api.github.com/users/mdsol/repos","events_url":"https://api.github.com/users/mdsol/events{/privacy}","received_events_url":"https://api.github.com/users/mdsol/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mdsol/the_policy_machine","description":"","fork":false,"url":"https://api.github.com/repos/mdsol/the_policy_machine","forks_url":"https://api.github.com/repos/mdsol/the_policy_machine/forks","keys_url":"https://api.github.com/repos/mdsol/the_policy_machine/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mdsol/the_policy_machine/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mdsol/the_policy_machine/teams","hooks_url":"https://api.github.com/repos/mdsol/the_policy_machine/hooks","issue_events_url":"https://api.github.com/repos/mdsol/the_policy_machine/issues/events{/number}","events_url":"https://api.github.com/repos/mdsol/the_policy_machine/events","assignees_url":"https://api.github.com/repos/mdsol/the_policy_machine/assignees{/user}","branches_url":"https://api.github.com/repos/mdsol/the_policy_machine/branches{/branch}","tags_url":"https://api.github.com/repos/mdsol/the_policy_machine/tags","blobs_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/refs{/sha}","trees_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mdsol/the_policy_machine/statuses/{sha}","languages_url":"https://api.github.com/repos/mdsol/the_policy_machine/languages","stargazers_url":"https://api.github.com/repos/mdsol/the_policy_machine/stargazers","contributors_url":"https://api.github.com/repos/mdsol/the_policy_machine/contributors","subscribers_url":"https://api.github.com/repos/mdsol/the_policy_machine/subscribers","subscription_url":"https://api.github.com/repos/mdsol/the_policy_machine/subscription","commits_url":"https://api.github.com/repos/mdsol/the_policy_machine/commits{/sha}","git_commits_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/commits{/sha}","comments_url":"https://api.github.com/repos/mdsol/the_policy_machine/comments{/number}","issue_comment_url":"https://api.github.com/repos/mdsol/the_policy_machine/issues/comments{/number}","contents_url":"https://api.github.com/repos/mdsol/the_policy_machine/contents/{+path}","compare_url":"https://api.github.com/repos/mdsol/the_policy_machine/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mdsol/the_policy_machine/merges","archive_url":"https://api.github.com/repos/mdsol/the_policy_machine/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mdsol/the_policy_machine/downloads","issues_url":"https://api.github.com/repos/mdsol/the_policy_machine/issues{/number}","pulls_url":"https://api.github.com/repos/mdsol/the_policy_machine/pulls{/number}","milestones_url":"https://api.github.com/repos/mdsol/the_policy_machine/milestones{/number}","notifications_url":"https://api.github.com/repos/mdsol/the_policy_machine/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mdsol/the_policy_machine/labels{/name}","releases_url":"https://api.github.com/repos/mdsol/the_policy_machine/releases{/id}","deployments_url":"https://api.github.com/repos/mdsol/the_policy_machine/deployments","created_at":"2013-10-18T18:30:56Z","updated_at":"2016-01-26T23:18:54Z","pushed_at":"2016-04-07T20:57:23Z","git_url":"git://github.com/mdsol/the_policy_machine.git","ssh_url":"git@github.com:mdsol/the_policy_machine.git","clone_url":"https://github.com/mdsol/the_policy_machine.git","svn_url":"https://github.com/mdsol/the_policy_machine","homepage":null,"size":178,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":1,"forks":0,"open_issues":1,"watchers":0,"default_branch":"develop"}},"base":{"label":"mdsol:develop","ref":"develop","sha":"2a261ccaad6e89cb5ae35ca61499496427959584","user":{"login":"mdsol","id":171103,"avatar_url":"https://avatars.githubusercontent.com/u/171103?v=3","gravatar_id":"","url":"https://api.github.com/users/mdsol","html_url":"https://github.com/mdsol","followers_url":"https://api.github.com/users/mdsol/followers","following_url":"https://api.github.com/users/mdsol/following{/other_user}","gists_url":"https://api.github.com/users/mdsol/gists{/gist_id}","starred_url":"https://api.github.com/users/mdsol/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mdsol/subscriptions","organizations_url":"https://api.github.com/users/mdsol/orgs","repos_url":"https://api.github.com/users/mdsol/repos","events_url":"https://api.github.com/users/mdsol/events{/privacy}","received_events_url":"https://api.github.com/users/mdsol/received_events","type":"Organization","site_admin":false},"repo":{"id":13685902,"name":"the_policy_machine","full_name":"mdsol/the_policy_machine","owner":{"login":"mdsol","id":171103,"avatar_url":"https://avatars.githubusercontent.com/u/171103?v=3","gravatar_id":"","url":"https://api.github.com/users/mdsol","html_url":"https://github.com/mdsol","followers_url":"https://api.github.com/users/mdsol/followers","following_url":"https://api.github.com/users/mdsol/following{/other_user}","gists_url":"https://api.github.com/users/mdsol/gists{/gist_id}","starred_url":"https://api.github.com/users/mdsol/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mdsol/subscriptions","organizations_url":"https://api.github.com/users/mdsol/orgs","repos_url":"https://api.github.com/users/mdsol/repos","events_url":"https://api.github.com/users/mdsol/events{/privacy}","received_events_url":"https://api.github.com/users/mdsol/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mdsol/the_policy_machine","description":"","fork":false,"url":"https://api.github.com/repos/mdsol/the_policy_machine","forks_url":"https://api.github.com/repos/mdsol/the_policy_machine/forks","keys_url":"https://api.github.com/repos/mdsol/the_policy_machine/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mdsol/the_policy_machine/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mdsol/the_policy_machine/teams","hooks_url":"https://api.github.com/repos/mdsol/the_policy_machine/hooks","issue_events_url":"https://api.github.com/repos/mdsol/the_policy_machine/issues/events{/number}","events_url":"https://api.github.com/repos/mdsol/the_policy_machine/events","assignees_url":"https://api.github.com/repos/mdsol/the_policy_machine/assignees{/user}","branches_url":"https://api.github.com/repos/mdsol/the_policy_machine/branches{/branch}","tags_url":"https://api.github.com/repos/mdsol/the_policy_machine/tags","blobs_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/refs{/sha}","trees_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mdsol/the_policy_machine/statuses/{sha}","languages_url":"https://api.github.com/repos/mdsol/the_policy_machine/languages","stargazers_url":"https://api.github.com/repos/mdsol/the_policy_machine/stargazers","contributors_url":"https://api.github.com/repos/mdsol/the_policy_machine/contributors","subscribers_url":"https://api.github.com/repos/mdsol/the_policy_machine/subscribers","subscription_url":"https://api.github.com/repos/mdsol/the_policy_machine/subscription","commits_url":"https://api.github.com/repos/mdsol/the_policy_machine/commits{/sha}","git_commits_url":"https://api.github.com/repos/mdsol/the_policy_machine/git/commits{/sha}","comments_url":"https://api.github.com/repos/mdsol/the_policy_machine/comments{/number}","issue_comment_url":"https://api.github.com/repos/mdsol/the_policy_machine/issues/comments{/number}","contents_url":"https://api.github.com/repos/mdsol/the_policy_machine/contents/{+path}","compare_url":"https://api.github.com/repos/mdsol/the_policy_machine/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mdsol/the_policy_machine/merges","archive_url":"https://api.github.com/repos/mdsol/the_policy_machine/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mdsol/the_policy_machine/downloads","issues_url":"https://api.github.com/repos/mdsol/the_policy_machine/issues{/number}","pulls_url":"https://api.github.com/repos/mdsol/the_policy_machine/pulls{/number}","milestones_url":"https://api.github.com/repos/mdsol/the_policy_machine/milestones{/number}","notifications_url":"https://api.github.com/repos/mdsol/the_policy_machine/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mdsol/the_policy_machine/labels{/name}","releases_url":"https://api.github.com/repos/mdsol/the_policy_machine/releases{/id}","deployments_url":"https://api.github.com/repos/mdsol/the_policy_machine/deployments","created_at":"2013-10-18T18:30:56Z","updated_at":"2016-01-26T23:18:54Z","pushed_at":"2016-04-07T20:57:23Z","git_url":"git://github.com/mdsol/the_policy_machine.git","ssh_url":"git@github.com:mdsol/the_policy_machine.git","clone_url":"https://github.com/mdsol/the_policy_machine.git","svn_url":"https://github.com/mdsol/the_policy_machine","homepage":null,"size":178,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":1,"forks":0,"open_issues":1,"watchers":0,"default_branch":"develop"}},"_links":{"self":{"href":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/54"},"html":{"href":"https://github.com/mdsol/the_policy_machine/pull/54"},"issue":{"href":"https://api.github.com/repos/mdsol/the_policy_machine/issues/54"},"comments":{"href":"https://api.github.com/repos/mdsol/the_policy_machine/issues/54/comments"},"review_comments":{"href":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/54/comments"},"review_comment":{"href":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mdsol/the_policy_machine/pulls/54/commits"},"statuses":{"href":"https://api.github.com/repos/mdsol/the_policy_machine/statuses/ac2cd1d8c7ba8ed82af966b469c3594952de2241"}}}}