GitHub PullRequestReviewCommentEvent at 2014-11-21 15:50:31
Repository: slamengine
Actor: jdegoes
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/slamdata/slamengine/pulls/comments/20722072","id":20722072,"diff_hunk":"@@ -292,6 +251,9 @@ object MongoDbPlanner extends Planner[Workflow] {\n     }\n   }\n \n+  type Dir = List[Int]\n+  type PartialSelector = (PartialFunction[List[BsonField], Selector], List[Dir])","path":"src/main/scala/slamdata/engine/physical/mongodb/planner.scala","position":103,"original_position":103,"commit_id":"19cc0d30c9c52011fba174ce34604718d788ddb7","original_commit_id":"19cc0d30c9c52011fba174ce34604718d788ddb7","user":{"login":"jdegoes","id":156745,"avatar_url":"https://avatars.githubusercontent.com/u/156745?v=3","gravatar_id":"","url":"https://api.github.com/users/jdegoes","html_url":"https://github.com/jdegoes","followers_url":"https://api.github.com/users/jdegoes/followers","following_url":"https://api.github.com/users/jdegoes/following{/other_user}","gists_url":"https://api.github.com/users/jdegoes/gists{/gist_id}","starred_url":"https://api.github.com/users/jdegoes/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdegoes/subscriptions","organizations_url":"https://api.github.com/users/jdegoes/orgs","repos_url":"https://api.github.com/users/jdegoes/repos","events_url":"https://api.github.com/users/jdegoes/events{/privacy}","received_events_url":"https://api.github.com/users/jdegoes/received_events","type":"User","site_admin":false},"body":"I'd argue the need for a comment is a sign the type is too specific or too broad. :imp: ","created_at":"2014-11-21T15:50:31Z","updated_at":"2014-11-21T15:50:31Z","html_url":"https://github.com/slamdata/slamengine/pull/456#discussion_r20722072","pull_request_url":"https://api.github.com/repos/slamdata/slamengine/pulls/456","_links":{"self":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/comments/20722072"},"html":{"href":"https://github.com/slamdata/slamengine/pull/456#discussion_r20722072"},"pull_request":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/456"}}},"pull_request":{"url":"https://api.github.com/repos/slamdata/slamengine/pulls/456","id":24820978,"html_url":"https://github.com/slamdata/slamengine/pull/456","diff_url":"https://github.com/slamdata/slamengine/pull/456.diff","patch_url":"https://github.com/slamdata/slamengine/pull/456.patch","issue_url":"https://api.github.com/repos/slamdata/slamengine/issues/456","number":456,"state":"open","locked":false,"title":"Implement `filter`.","user":{"login":"sellout","id":33031,"avatar_url":"https://avatars.githubusercontent.com/u/33031?v=3","gravatar_id":"","url":"https://api.github.com/users/sellout","html_url":"https://github.com/sellout","followers_url":"https://api.github.com/users/sellout/followers","following_url":"https://api.github.com/users/sellout/following{/other_user}","gists_url":"https://api.github.com/users/sellout/gists{/gist_id}","starred_url":"https://api.github.com/users/sellout/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sellout/subscriptions","organizations_url":"https://api.github.com/users/sellout/orgs","repos_url":"https://api.github.com/users/sellout/repos","events_url":"https://api.github.com/users/sellout/events{/privacy}","received_events_url":"https://api.github.com/users/sellout/received_events","type":"User","site_admin":false},"body":"This is not a complete fix for #207, but it does implement `filter` and\nallow certain cases of `HAVING`, etc. to work. However, there are other\ncases that still fail for other reasons: bad merging, broken\n`$foldLeft`, …\n\nSelectors no longer use the FieldPhase (which has been deleted), but now\nkeep track of the nodes they need WorkflowBuilders for.\n\n* make `appendOp` private, and add explicit `skip` and `limit` functions;\n* implement arithmetic in JsExprPhase;\n* improve WorkflowBuilder merging in a few cases;\n* remove JsExprPhase dependency from SelectorPhase (WorkflowPhase now\n  handles generating `$where` selectors);\n* reduce instances of $foldLeft (via converting a `$map` to a\n  `$simpleMap`);\n* eliminate `$projects` and `__tmp`s in many places (via eliminating use\n  of `Makearray` for sort and group keys, as well as some inlining of\n  ExprBuilders in some cases);\n* the above also means that more `select *` queries have a result at\n  `$$ROOT` rather than `value`;\n* prevent deleteUnusedFields from deleting used fields in the case that a\n  shape-preserving op preceded an unknown shape op (like $map) or the\n  end of the query (this became apparent when the final `$projects`\n  disappeared as mentioned above)\n\nThis also improves the request generated for #447, but it’s uncertain\nwhether that is actually fixed.","created_at":"2014-11-21T06:08:24Z","updated_at":"2014-11-21T15:50:31Z","closed_at":null,"merged_at":null,"merge_commit_sha":"45cf34dc1771eef7b446c6fab1b0374391fdb8ef","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/slamdata/slamengine/pulls/456/commits","review_comments_url":"https://api.github.com/repos/slamdata/slamengine/pulls/456/comments","review_comment_url":"https://api.github.com/repos/slamdata/slamengine/pulls/comments/{number}","comments_url":"https://api.github.com/repos/slamdata/slamengine/issues/456/comments","statuses_url":"https://api.github.com/repos/slamdata/slamengine/statuses/19cc0d30c9c52011fba174ce34604718d788ddb7","head":{"label":"sellout:ready/207-fix-filtering-w-new-WB","ref":"ready/207-fix-filtering-w-new-WB","sha":"19cc0d30c9c52011fba174ce34604718d788ddb7","user":{"login":"sellout","id":33031,"avatar_url":"https://avatars.githubusercontent.com/u/33031?v=3","gravatar_id":"","url":"https://api.github.com/users/sellout","html_url":"https://github.com/sellout","followers_url":"https://api.github.com/users/sellout/followers","following_url":"https://api.github.com/users/sellout/following{/other_user}","gists_url":"https://api.github.com/users/sellout/gists{/gist_id}","starred_url":"https://api.github.com/users/sellout/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sellout/subscriptions","organizations_url":"https://api.github.com/users/sellout/orgs","repos_url":"https://api.github.com/users/sellout/repos","events_url":"https://api.github.com/users/sellout/events{/privacy}","received_events_url":"https://api.github.com/users/sellout/received_events","type":"User","site_admin":false},"repo":{"id":21178230,"name":"slamengine","full_name":"sellout/slamengine","owner":{"login":"sellout","id":33031,"avatar_url":"https://avatars.githubusercontent.com/u/33031?v=3","gravatar_id":"","url":"https://api.github.com/users/sellout","html_url":"https://github.com/sellout","followers_url":"https://api.github.com/users/sellout/followers","following_url":"https://api.github.com/users/sellout/following{/other_user}","gists_url":"https://api.github.com/users/sellout/gists{/gist_id}","starred_url":"https://api.github.com/users/sellout/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sellout/subscriptions","organizations_url":"https://api.github.com/users/sellout/orgs","repos_url":"https://api.github.com/users/sellout/repos","events_url":"https://api.github.com/users/sellout/events{/privacy}","received_events_url":"https://api.github.com/users/sellout/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/sellout/slamengine","description":"The engine for SlamData","fork":true,"url":"https://api.github.com/repos/sellout/slamengine","forks_url":"https://api.github.com/repos/sellout/slamengine/forks","keys_url":"https://api.github.com/repos/sellout/slamengine/keys{/key_id}","collaborators_url":"https://api.github.com/repos/sellout/slamengine/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/sellout/slamengine/teams","hooks_url":"https://api.github.com/repos/sellout/slamengine/hooks","issue_events_url":"https://api.github.com/repos/sellout/slamengine/issues/events{/number}","events_url":"https://api.github.com/repos/sellout/slamengine/events","assignees_url":"https://api.github.com/repos/sellout/slamengine/assignees{/user}","branches_url":"https://api.github.com/repos/sellout/slamengine/branches{/branch}","tags_url":"https://api.github.com/repos/sellout/slamengine/tags","blobs_url":"https://api.github.com/repos/sellout/slamengine/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/sellout/slamengine/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/sellout/slamengine/git/refs{/sha}","trees_url":"https://api.github.com/repos/sellout/slamengine/git/trees{/sha}","statuses_url":"https://api.github.com/repos/sellout/slamengine/statuses/{sha}","languages_url":"https://api.github.com/repos/sellout/slamengine/languages","stargazers_url":"https://api.github.com/repos/sellout/slamengine/stargazers","contributors_url":"https://api.github.com/repos/sellout/slamengine/contributors","subscribers_url":"https://api.github.com/repos/sellout/slamengine/subscribers","subscription_url":"https://api.github.com/repos/sellout/slamengine/subscription","commits_url":"https://api.github.com/repos/sellout/slamengine/commits{/sha}","git_commits_url":"https://api.github.com/repos/sellout/slamengine/git/commits{/sha}","comments_url":"https://api.github.com/repos/sellout/slamengine/comments{/number}","issue_comment_url":"https://api.github.com/repos/sellout/slamengine/issues/comments/{number}","contents_url":"https://api.github.com/repos/sellout/slamengine/contents/{+path}","compare_url":"https://api.github.com/repos/sellout/slamengine/compare/{base}...{head}","merges_url":"https://api.github.com/repos/sellout/slamengine/merges","archive_url":"https://api.github.com/repos/sellout/slamengine/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/sellout/slamengine/downloads","issues_url":"https://api.github.com/repos/sellout/slamengine/issues{/number}","pulls_url":"https://api.github.com/repos/sellout/slamengine/pulls{/number}","milestones_url":"https://api.github.com/repos/sellout/slamengine/milestones{/number}","notifications_url":"https://api.github.com/repos/sellout/slamengine/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/sellout/slamengine/labels{/name}","releases_url":"https://api.github.com/repos/sellout/slamengine/releases{/id}","created_at":"2014-06-24T19:38:07Z","updated_at":"2014-09-08T15:39:05Z","pushed_at":"2014-11-21T06:08:15Z","git_url":"git://github.com/sellout/slamengine.git","ssh_url":"git@github.com:sellout/slamengine.git","clone_url":"https://github.com/sellout/slamengine.git","svn_url":"https://github.com/sellout/slamengine","homepage":null,"size":4384,"stargazers_count":0,"watchers_count":0,"language":"Scala","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"CHECK-UPSTREAM-REPO"}},"base":{"label":"slamdata:master","ref":"master","sha":"63f5f206cffb46572bf6b5ea962e97c2db0ebfe9","user":{"login":"slamdata","id":5385602,"avatar_url":"https://avatars.githubusercontent.com/u/5385602?v=3","gravatar_id":"","url":"https://api.github.com/users/slamdata","html_url":"https://github.com/slamdata","followers_url":"https://api.github.com/users/slamdata/followers","following_url":"https://api.github.com/users/slamdata/following{/other_user}","gists_url":"https://api.github.com/users/slamdata/gists{/gist_id}","starred_url":"https://api.github.com/users/slamdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/slamdata/subscriptions","organizations_url":"https://api.github.com/users/slamdata/orgs","repos_url":"https://api.github.com/users/slamdata/repos","events_url":"https://api.github.com/users/slamdata/events{/privacy}","received_events_url":"https://api.github.com/users/slamdata/received_events","type":"Organization","site_admin":false},"repo":{"id":16515058,"name":"slamengine","full_name":"slamdata/slamengine","owner":{"login":"slamdata","id":5385602,"avatar_url":"https://avatars.githubusercontent.com/u/5385602?v=3","gravatar_id":"","url":"https://api.github.com/users/slamdata","html_url":"https://github.com/slamdata","followers_url":"https://api.github.com/users/slamdata/followers","following_url":"https://api.github.com/users/slamdata/following{/other_user}","gists_url":"https://api.github.com/users/slamdata/gists{/gist_id}","starred_url":"https://api.github.com/users/slamdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/slamdata/subscriptions","organizations_url":"https://api.github.com/users/slamdata/orgs","repos_url":"https://api.github.com/users/slamdata/repos","events_url":"https://api.github.com/users/slamdata/events{/privacy}","received_events_url":"https://api.github.com/users/slamdata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/slamdata/slamengine","description":"The engine for SlamData","fork":false,"url":"https://api.github.com/repos/slamdata/slamengine","forks_url":"https://api.github.com/repos/slamdata/slamengine/forks","keys_url":"https://api.github.com/repos/slamdata/slamengine/keys{/key_id}","collaborators_url":"https://api.github.com/repos/slamdata/slamengine/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/slamdata/slamengine/teams","hooks_url":"https://api.github.com/repos/slamdata/slamengine/hooks","issue_events_url":"https://api.github.com/repos/slamdata/slamengine/issues/events{/number}","events_url":"https://api.github.com/repos/slamdata/slamengine/events","assignees_url":"https://api.github.com/repos/slamdata/slamengine/assignees{/user}","branches_url":"https://api.github.com/repos/slamdata/slamengine/branches{/branch}","tags_url":"https://api.github.com/repos/slamdata/slamengine/tags","blobs_url":"https://api.github.com/repos/slamdata/slamengine/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/slamdata/slamengine/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/slamdata/slamengine/git/refs{/sha}","trees_url":"https://api.github.com/repos/slamdata/slamengine/git/trees{/sha}","statuses_url":"https://api.github.com/repos/slamdata/slamengine/statuses/{sha}","languages_url":"https://api.github.com/repos/slamdata/slamengine/languages","stargazers_url":"https://api.github.com/repos/slamdata/slamengine/stargazers","contributors_url":"https://api.github.com/repos/slamdata/slamengine/contributors","subscribers_url":"https://api.github.com/repos/slamdata/slamengine/subscribers","subscription_url":"https://api.github.com/repos/slamdata/slamengine/subscription","commits_url":"https://api.github.com/repos/slamdata/slamengine/commits{/sha}","git_commits_url":"https://api.github.com/repos/slamdata/slamengine/git/commits{/sha}","comments_url":"https://api.github.com/repos/slamdata/slamengine/comments{/number}","issue_comment_url":"https://api.github.com/repos/slamdata/slamengine/issues/comments/{number}","contents_url":"https://api.github.com/repos/slamdata/slamengine/contents/{+path}","compare_url":"https://api.github.com/repos/slamdata/slamengine/compare/{base}...{head}","merges_url":"https://api.github.com/repos/slamdata/slamengine/merges","archive_url":"https://api.github.com/repos/slamdata/slamengine/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/slamdata/slamengine/downloads","issues_url":"https://api.github.com/repos/slamdata/slamengine/issues{/number}","pulls_url":"https://api.github.com/repos/slamdata/slamengine/pulls{/number}","milestones_url":"https://api.github.com/repos/slamdata/slamengine/milestones{/number}","notifications_url":"https://api.github.com/repos/slamdata/slamengine/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/slamdata/slamengine/labels{/name}","releases_url":"https://api.github.com/repos/slamdata/slamengine/releases{/id}","created_at":"2014-02-04T15:01:27Z","updated_at":"2014-11-21T02:51:33Z","pushed_at":"2014-11-21T03:23:57Z","git_url":"git://github.com/slamdata/slamengine.git","ssh_url":"git@github.com:slamdata/slamengine.git","clone_url":"https://github.com/slamdata/slamengine.git","svn_url":"https://github.com/slamdata/slamengine","homepage":null,"size":9088,"stargazers_count":57,"watchers_count":57,"language":"Scala","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":16,"mirror_url":null,"open_issues_count":102,"forks":16,"open_issues":102,"watchers":57,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/456"},"html":{"href":"https://github.com/slamdata/slamengine/pull/456"},"issue":{"href":"https://api.github.com/repos/slamdata/slamengine/issues/456"},"comments":{"href":"https://api.github.com/repos/slamdata/slamengine/issues/456/comments"},"review_comments":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/456/comments"},"review_comment":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/456/commits"},"statuses":{"href":"https://api.github.com/repos/slamdata/slamengine/statuses/19cc0d30c9c52011fba174ce34604718d788ddb7"}}}}