{"action":"created","comment":{"url":"https://api.github.com/repos/slamdata/slamengine/pulls/comments/39215148","id":39215148,"diff_hunk":"@@ -615,35 +604,66 @@ trait Compiler[F[_]] {\n \n           case NullLiteral() => emit(LogicalPlan.Constant(Data.Null))\n         }\n-\n-        for {\n-          nodeLP  <- nodeLP\n-          root    <- CompilerState.rootTable\n-          grouped <- CompilerState.grouped\n-        } yield (root |@| grouped) { (root, grouped) =>\n-          val nodeLP1 = nodeLP.rewrite {\n-            case `root` => Some(grouped.src)\n-            case _      => None\n-          }\n-          if (grouped.keys contains nodeLP1)\n-            Some(LogicalPlan.Invoke(agg.Arbitrary, List(nodeLP)))\n-          else\n-            None\n-        }.join.getOrElse(nodeLP)\n     })\n   }\n \n   def compile(tree: AnnotatedTree[Expr, Annotations])(implicit F: Monad[F]): F[SemanticError \\/ Fix[LogicalPlan]] = {\n-    compile0(tree.root).eval(CompilerState(tree, Nil, Nil, 0, None)).run\n+    compile0(tree.root).eval(CompilerState(tree, Nil, Nil, 0)).run.map(_.map(Compiler.reduceGroupKeys))\n   }\n }\n \n object Compiler {\n+  import LogicalPlan._\n+\n   def apply[F[_]]: Compiler[F] = new Compiler[F] {}\n \n-  def trampoline = apply[Free.Trampoline]\n+  def trampoline = apply[scalaz.Free.Trampoline]\n \n   def compile(tree: AnnotatedTree[Expr, Annotations]): SemanticError \\/ Fix[LogicalPlan] = {\n     trampoline.compile(tree).run\n   }\n+\n+  def reduceGroupKeys(tree: Fix[LogicalPlan]): Fix[LogicalPlan] = {\n+    // Step 0: identify key expressions, and rewrite them by replacing the\n+    // group source with the source at the point where they might appear.\n+    def keysƒ(t: LogicalPlan[(Fix[LogicalPlan], List[Fix[LogicalPlan]])]):\n+        (Fix[LogicalPlan], List[Fix[LogicalPlan]]) =\n+    {\n+      def groupedKeys(t: LogicalPlan[Fix[LogicalPlan]], newSrc: Fix[LogicalPlan]): Option[List[Fix[LogicalPlan]]] = {\n+        t match {\n+          case InvokeF(set.GroupBy, List(src, structural.MakeArrayN(keys))) =>\n+            Some(keys.map(_.transform(t => if (t == src) newSrc else t)))\n+          case InvokeF(func, src :: _) if func.mappingType == MappingType.ManyToMany =>\n+            groupedKeys(src.unFix, newSrc)\n+          case _ => None\n+        }\n+      }\n+\n+      Fix(t.map(_._1)) ->\n+        groupedKeys(t.map(_._1), Fix(t.map(_._1))).getOrElse(t.foldMap(_._2))\n+    }\n+    val keys: List[Fix[LogicalPlan]] = boundCata(tree)(keysƒ)._2","path":"core/src/main/scala/slamdata/engine/compiler.scala","position":113,"original_position":113,"commit_id":"c1809aee7a39204aee14280ff5bee7332d954586","original_commit_id":"c1809aee7a39204aee14280ff5bee7332d954586","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":"Ah, right … `boundPara` gives you the `Free` in the `Fix[F]`, only giving you the `A` from the bound value. I remember hemming and hawing over it for a while.\r\n\r\nThe reason it landed where it did is because in something like `preferProjectionsƒ`, where `A` is `Fix[F]`, you probably want to be able to preserve at least some of the variable references in the result.","created_at":"2015-09-10T21:17:46Z","updated_at":"2015-09-10T21:17:46Z","html_url":"https://github.com/slamdata/slamengine/pull/911#discussion_r39215148","pull_request_url":"https://api.github.com/repos/slamdata/slamengine/pulls/911","_links":{"self":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/comments/39215148"},"html":{"href":"https://github.com/slamdata/slamengine/pull/911#discussion_r39215148"},"pull_request":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/911"}}},"pull_request":{"url":"https://api.github.com/repos/slamdata/slamengine/pulls/911","id":44497340,"html_url":"https://github.com/slamdata/slamengine/pull/911","diff_url":"https://github.com/slamdata/slamengine/pull/911.diff","patch_url":"https://github.com/slamdata/slamengine/pull/911.patch","issue_url":"https://api.github.com/repos/slamdata/slamengine/issues/911","number":911,"state":"open","locked":false,"title":"Avoid adding Arbitrary when group keys are already reduced","user":{"login":"mossprescott","id":6147371,"avatar_url":"https://avatars.githubusercontent.com/u/6147371?v=3","gravatar_id":"","url":"https://api.github.com/users/mossprescott","html_url":"https://github.com/mossprescott","followers_url":"https://api.github.com/users/mossprescott/followers","following_url":"https://api.github.com/users/mossprescott/following{/other_user}","gists_url":"https://api.github.com/users/mossprescott/gists{/gist_id}","starred_url":"https://api.github.com/users/mossprescott/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mossprescott/subscriptions","organizations_url":"https://api.github.com/users/mossprescott/orgs","repos_url":"https://api.github.com/users/mossprescott/repos","events_url":"https://api.github.com/users/mossprescott/events{/privacy}","received_events_url":"https://api.github.com/users/mossprescott/received_events","type":"User","site_admin":false},"body":"Resolves SD-898.\n\nAdd a transformation of the LogicalPlan after is is generated from the SQL\nAST and before returning it from the compiler. Implement it as a series of\nsteps: identifying keys, annotating expressions that match the keys,\ninserting Arbitrary where no reduction is already present. Two of those\nsteps use new, fairly generic, folds: `topDownTransform` (for `Cofree`) and\n`boundParaAttribute`.\n\nRemove the previous approach to inserting Arbitrary, which added the keys\nto the compiler's state.\n\nShape-preserving functions are now identified with a separate mappingType\nfrom other transformations (grouping and joins), so that keys can be\ntracked when filtering happens in between grouping and projections (e.g.\nSQL's `HAVING`).\n\nFor debugging, added RenderTree instances for several primitive types.","created_at":"2015-09-09T00:01:56Z","updated_at":"2015-09-10T21:17:46Z","closed_at":null,"merged_at":null,"merge_commit_sha":"2c2e50e057186ad70fa0a9ae9b6e2bcf441c6375","assignee":{"login":"wemrysi","id":32185,"avatar_url":"https://avatars.githubusercontent.com/u/32185?v=3","gravatar_id":"","url":"https://api.github.com/users/wemrysi","html_url":"https://github.com/wemrysi","followers_url":"https://api.github.com/users/wemrysi/followers","following_url":"https://api.github.com/users/wemrysi/following{/other_user}","gists_url":"https://api.github.com/users/wemrysi/gists{/gist_id}","starred_url":"https://api.github.com/users/wemrysi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wemrysi/subscriptions","organizations_url":"https://api.github.com/users/wemrysi/orgs","repos_url":"https://api.github.com/users/wemrysi/repos","events_url":"https://api.github.com/users/wemrysi/events{/privacy}","received_events_url":"https://api.github.com/users/wemrysi/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/slamdata/slamengine/pulls/911/commits","review_comments_url":"https://api.github.com/repos/slamdata/slamengine/pulls/911/comments","review_comment_url":"https://api.github.com/repos/slamdata/slamengine/pulls/comments{/number}","comments_url":"https://api.github.com/repos/slamdata/slamengine/issues/911/comments","statuses_url":"https://api.github.com/repos/slamdata/slamengine/statuses/c1809aee7a39204aee14280ff5bee7332d954586","head":{"label":"mossprescott:ready/SD-898","ref":"ready/SD-898","sha":"c1809aee7a39204aee14280ff5bee7332d954586","user":{"login":"mossprescott","id":6147371,"avatar_url":"https://avatars.githubusercontent.com/u/6147371?v=3","gravatar_id":"","url":"https://api.github.com/users/mossprescott","html_url":"https://github.com/mossprescott","followers_url":"https://api.github.com/users/mossprescott/followers","following_url":"https://api.github.com/users/mossprescott/following{/other_user}","gists_url":"https://api.github.com/users/mossprescott/gists{/gist_id}","starred_url":"https://api.github.com/users/mossprescott/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mossprescott/subscriptions","organizations_url":"https://api.github.com/users/mossprescott/orgs","repos_url":"https://api.github.com/users/mossprescott/repos","events_url":"https://api.github.com/users/mossprescott/events{/privacy}","received_events_url":"https://api.github.com/users/mossprescott/received_events","type":"User","site_admin":false},"repo":{"id":21178367,"name":"slamengine","full_name":"mossprescott/slamengine","owner":{"login":"mossprescott","id":6147371,"avatar_url":"https://avatars.githubusercontent.com/u/6147371?v=3","gravatar_id":"","url":"https://api.github.com/users/mossprescott","html_url":"https://github.com/mossprescott","followers_url":"https://api.github.com/users/mossprescott/followers","following_url":"https://api.github.com/users/mossprescott/following{/other_user}","gists_url":"https://api.github.com/users/mossprescott/gists{/gist_id}","starred_url":"https://api.github.com/users/mossprescott/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mossprescott/subscriptions","organizations_url":"https://api.github.com/users/mossprescott/orgs","repos_url":"https://api.github.com/users/mossprescott/repos","events_url":"https://api.github.com/users/mossprescott/events{/privacy}","received_events_url":"https://api.github.com/users/mossprescott/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/mossprescott/slamengine","description":"The engine for SlamData","fork":true,"url":"https://api.github.com/repos/mossprescott/slamengine","forks_url":"https://api.github.com/repos/mossprescott/slamengine/forks","keys_url":"https://api.github.com/repos/mossprescott/slamengine/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mossprescott/slamengine/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mossprescott/slamengine/teams","hooks_url":"https://api.github.com/repos/mossprescott/slamengine/hooks","issue_events_url":"https://api.github.com/repos/mossprescott/slamengine/issues/events{/number}","events_url":"https://api.github.com/repos/mossprescott/slamengine/events","assignees_url":"https://api.github.com/repos/mossprescott/slamengine/assignees{/user}","branches_url":"https://api.github.com/repos/mossprescott/slamengine/branches{/branch}","tags_url":"https://api.github.com/repos/mossprescott/slamengine/tags","blobs_url":"https://api.github.com/repos/mossprescott/slamengine/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mossprescott/slamengine/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mossprescott/slamengine/git/refs{/sha}","trees_url":"https://api.github.com/repos/mossprescott/slamengine/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mossprescott/slamengine/statuses/{sha}","languages_url":"https://api.github.com/repos/mossprescott/slamengine/languages","stargazers_url":"https://api.github.com/repos/mossprescott/slamengine/stargazers","contributors_url":"https://api.github.com/repos/mossprescott/slamengine/contributors","subscribers_url":"https://api.github.com/repos/mossprescott/slamengine/subscribers","subscription_url":"https://api.github.com/repos/mossprescott/slamengine/subscription","commits_url":"https://api.github.com/repos/mossprescott/slamengine/commits{/sha}","git_commits_url":"https://api.github.com/repos/mossprescott/slamengine/git/commits{/sha}","comments_url":"https://api.github.com/repos/mossprescott/slamengine/comments{/number}","issue_comment_url":"https://api.github.com/repos/mossprescott/slamengine/issues/comments{/number}","contents_url":"https://api.github.com/repos/mossprescott/slamengine/contents/{+path}","compare_url":"https://api.github.com/repos/mossprescott/slamengine/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mossprescott/slamengine/merges","archive_url":"https://api.github.com/repos/mossprescott/slamengine/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mossprescott/slamengine/downloads","issues_url":"https://api.github.com/repos/mossprescott/slamengine/issues{/number}","pulls_url":"https://api.github.com/repos/mossprescott/slamengine/pulls{/number}","milestones_url":"https://api.github.com/repos/mossprescott/slamengine/milestones{/number}","notifications_url":"https://api.github.com/repos/mossprescott/slamengine/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mossprescott/slamengine/labels{/name}","releases_url":"https://api.github.com/repos/mossprescott/slamengine/releases{/id}","created_at":"2014-06-24T19:43:22Z","updated_at":"2015-01-08T17:19:20Z","pushed_at":"2015-09-09T21:00:30Z","git_url":"git://github.com/mossprescott/slamengine.git","ssh_url":"git@github.com:mossprescott/slamengine.git","clone_url":"https://github.com/mossprescott/slamengine.git","svn_url":"https://github.com/mossprescott/slamengine","homepage":null,"size":7952,"stargazers_count":0,"watchers_count":0,"language":"Scala","has_issues":false,"has_downloads":true,"has_wiki":true,"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":"b28b4a7afe493ee47f76169126e73ea523e76488","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":"A NoSQL analytics engine that embraces post-relational analytics and pushes computation to the data.","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":"2015-09-08T00:51:08Z","pushed_at":"2015-09-10T19:55:12Z","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":"","size":10249,"stargazers_count":141,"watchers_count":141,"language":"Scala","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":36,"mirror_url":null,"open_issues_count":162,"forks":36,"open_issues":162,"watchers":141,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/911"},"html":{"href":"https://github.com/slamdata/slamengine/pull/911"},"issue":{"href":"https://api.github.com/repos/slamdata/slamengine/issues/911"},"comments":{"href":"https://api.github.com/repos/slamdata/slamengine/issues/911/comments"},"review_comments":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/911/comments"},"review_comment":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/slamdata/slamengine/pulls/911/commits"},"statuses":{"href":"https://api.github.com/repos/slamdata/slamengine/statuses/c1809aee7a39204aee14280ff5bee7332d954586"}}}}