{"action":"created","comment":{"url":"https://api.github.com/repos/facebook/presto/pulls/comments/35180166","id":35180166,"diff_hunk":"@@ -60,220 +64,149 @@ public PlanNode optimize(PlanNode plan, Session session, Map<Symbol, Type> types\n         checkNotNull(symbolAllocator, \"symbolAllocator is null\");\n         checkNotNull(idAllocator, \"idAllocator is null\");\n \n-        return PlanRewriter.rewriteWith(new Rewriter(idAllocator), plan, null);\n+        return PlanRewriter.rewriteWith(new Rewriter(idAllocator, symbolAllocator, metadata, session), plan, null);\n     }\n \n     private static class Rewriter\n-            extends PlanRewriter<Constraint>\n+            extends PlanRewriter<Void>\n     {\n         private final PlanNodeIdAllocator idAllocator;\n+        private final SymbolAllocator symbolAllocator;\n+        private final Metadata metadata;\n+        private final Session session;\n \n-        private Rewriter(PlanNodeIdAllocator idAllocator)\n+        private Rewriter(PlanNodeIdAllocator idAllocator, SymbolAllocator symbolAllocator, Metadata metadata, Session session)\n         {\n-            this.idAllocator = checkNotNull(idAllocator, \"idAllocator is null\");\n+            this.idAllocator = requireNonNull(idAllocator, \"idAllocator is null\");\n+            this.symbolAllocator = requireNonNull(symbolAllocator, \"symbolAllocator is null\");\n+            this.metadata = requireNonNull(metadata, \"metadata is null\");\n+            this.session = requireNonNull(session, \"session is null\");\n         }\n \n         @Override\n-        public PlanNode visitWindow(WindowNode node, RewriteContext<Constraint> context)\n+        public PlanNode visitWindow(WindowNode node, RewriteContext<Void> context)\n         {\n-            if (canOptimizeWindowFunction(node)) {\n-                PlanNode rewrittenSource = context.rewrite(node.getSource(), null);\n-                Optional<Integer> limit = getLimit(node, context.get());\n-                if (node.getOrderBy().isEmpty()) {\n-                    return new RowNumberNode(idAllocator.getNextId(),\n-                            rewrittenSource,\n-                            node.getPartitionBy(),\n-                            getOnlyElement(node.getWindowFunctions().keySet()),\n-                            limit,\n-                            Optional.empty());\n-                }\n-                if (limit.isPresent()) {\n-                    return new TopNRowNumberNode(idAllocator.getNextId(),\n-                            rewrittenSource,\n-                            node.getPartitionBy(),\n-                            node.getOrderBy(),\n-                            node.getOrderings(),\n-                            getOnlyElement(node.getWindowFunctions().keySet()),\n-                            limit.get(),\n-                            false,\n-                            Optional.empty());\n-                }\n-            }\n-            return context.defaultRewrite(node);\n-        }\n+            PlanNode rewrittenSource = context.rewrite(node.getSource());\n \n-        private static Optional<Integer> getLimit(WindowNode node, Constraint filter)\n-        {\n-            if (filter == null || (!filter.getLimit().isPresent() && !filter.getFilterExpression().isPresent())) {\n-                return Optional.empty();\n+            if (canReplaceWithRowNumber(node)) {\n+                return new RowNumberNode(idAllocator.getNextId(),\n+                        rewrittenSource,\n+                        node.getPartitionBy(),\n+                        getOnlyElement(node.getWindowFunctions().keySet()),\n+                        Optional.empty(),\n+                        Optional.empty());\n             }\n-            if (filter.getLimit().isPresent()) {\n-                return filter.getLimit();\n-            }\n-            if (filterContainsWindowFunctions(node, filter.getFilterExpression().get()) &&\n-                    filter.getFilterExpression().get() instanceof ComparisonExpression) {\n-                Symbol rowNumberSymbol = Iterables.getOnlyElement(node.getWindowFunctions().entrySet()).getKey();\n-                return WindowLimitExtractor.extract(filter.getFilterExpression().get(), rowNumberSymbol);\n-            }\n-            return Optional.empty();\n-        }\n-\n-        private static boolean canOptimizeWindowFunction(WindowNode node)\n-        {\n-            if (node.getWindowFunctions().size() != 1) {\n-                return false;\n-            }\n-            Symbol rowNumberSymbol = getOnlyElement(node.getWindowFunctions().entrySet()).getKey();\n-            return isRowNumberSignature(node.getSignatures().get(rowNumberSymbol));\n-        }\n-\n-        private static boolean filterContainsWindowFunctions(WindowNode node, Expression filterPredicate)\n-        {\n-            Set<Symbol> windowFunctionSymbols = node.getWindowFunctions().keySet();\n-            Sets.SetView<Symbol> commonSymbols = Sets.intersection(DependencyExtractor.extractUnique(filterPredicate), windowFunctionSymbols);\n-            return !commonSymbols.isEmpty();\n+            return context.replaceChildren(node, ImmutableList.of(rewrittenSource));\n         }\n \n         @Override\n-        public PlanNode visitLimit(LimitNode node, RewriteContext<Constraint> context)\n+        public PlanNode visitLimit(LimitNode node, RewriteContext<Void> context)\n         {\n             // Operators can handle MAX_VALUE rows per page, so do not optimize if count is greater than this value\n             if (node.getCount() >= Integer.MAX_VALUE) {\n                 return context.defaultRewrite(node);\n             }\n-            Constraint constraint = new Constraint(Optional.of((int) node.getCount()), Optional.empty());\n-            PlanNode rewrittenSource = context.rewrite(node.getSource(), constraint);\n \n-            if (rewrittenSource != node.getSource()) {\n-                return new LimitNode(idAllocator.getNextId(), rewrittenSource, node.getCount());\n+            PlanNode rewrittenSource = context.rewrite(node.getSource());\n+            if (rewrittenSource instanceof RowNumberNode) {\n+                RowNumberNode rowNumberNode = (RowNumberNode) rewrittenSource;\n+                if (rowNumberNode.getPartitionBy().isEmpty()) {","path":"presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/WindowFilterPushDown.java","position":169,"original_position":169,"commit_id":"6a51258c484b61c97b2e2adce72b954ca3a4124c","original_commit_id":"6a51258c484b61c97b2e2adce72b954ca3a4124c","user":{"login":"nileema","id":1034143,"avatar_url":"https://avatars.githubusercontent.com/u/1034143?v=3","gravatar_id":"","url":"https://api.github.com/users/nileema","html_url":"https://github.com/nileema","followers_url":"https://api.github.com/users/nileema/followers","following_url":"https://api.github.com/users/nileema/following{/other_user}","gists_url":"https://api.github.com/users/nileema/gists{/gist_id}","starred_url":"https://api.github.com/users/nileema/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nileema/subscriptions","organizations_url":"https://api.github.com/users/nileema/orgs","repos_url":"https://api.github.com/users/nileema/repos","events_url":"https://api.github.com/users/nileema/events{/privacy}","received_events_url":"https://api.github.com/users/nileema/received_events","type":"User","site_admin":false},"body":"I'll change it so that it always adds a row number limit (or a topNRowNumber) and always retains the limit node on top of that (instead of squashing it into one node for unpartitioned row number). ","created_at":"2015-07-22T04:15:10Z","updated_at":"2015-07-22T04:15:10Z","html_url":"https://github.com/facebook/presto/pull/3323#discussion_r35180166","pull_request_url":"https://api.github.com/repos/facebook/presto/pulls/3323","_links":{"self":{"href":"https://api.github.com/repos/facebook/presto/pulls/comments/35180166"},"html":{"href":"https://github.com/facebook/presto/pull/3323#discussion_r35180166"},"pull_request":{"href":"https://api.github.com/repos/facebook/presto/pulls/3323"}}},"pull_request":{"url":"https://api.github.com/repos/facebook/presto/pulls/3323","id":40521744,"html_url":"https://github.com/facebook/presto/pull/3323","diff_url":"https://github.com/facebook/presto/pull/3323.diff","patch_url":"https://github.com/facebook/presto/pull/3323.patch","issue_url":"https://api.github.com/repos/facebook/presto/issues/3323","number":3323,"state":"open","locked":false,"title":"Fixes for WindowFilterPushDown","user":{"login":"nileema","id":1034143,"avatar_url":"https://avatars.githubusercontent.com/u/1034143?v=3","gravatar_id":"","url":"https://api.github.com/users/nileema","html_url":"https://github.com/nileema","followers_url":"https://api.github.com/users/nileema/followers","following_url":"https://api.github.com/users/nileema/following{/other_user}","gists_url":"https://api.github.com/users/nileema/gists{/gist_id}","starred_url":"https://api.github.com/users/nileema/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nileema/subscriptions","organizations_url":"https://api.github.com/users/nileema/orgs","repos_url":"https://api.github.com/users/nileema/repos","events_url":"https://api.github.com/users/nileema/events{/privacy}","received_events_url":"https://api.github.com/users/nileema/received_events","type":"User","site_admin":false},"body":"","created_at":"2015-07-22T01:12:58Z","updated_at":"2015-07-22T04:15:10Z","closed_at":null,"merged_at":null,"merge_commit_sha":"276398072e3efc46d287e1e697a03f4fcbfac1db","assignee":{"login":"erichwang","id":737851,"avatar_url":"https://avatars.githubusercontent.com/u/737851?v=3","gravatar_id":"","url":"https://api.github.com/users/erichwang","html_url":"https://github.com/erichwang","followers_url":"https://api.github.com/users/erichwang/followers","following_url":"https://api.github.com/users/erichwang/following{/other_user}","gists_url":"https://api.github.com/users/erichwang/gists{/gist_id}","starred_url":"https://api.github.com/users/erichwang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/erichwang/subscriptions","organizations_url":"https://api.github.com/users/erichwang/orgs","repos_url":"https://api.github.com/users/erichwang/repos","events_url":"https://api.github.com/users/erichwang/events{/privacy}","received_events_url":"https://api.github.com/users/erichwang/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/facebook/presto/pulls/3323/commits","review_comments_url":"https://api.github.com/repos/facebook/presto/pulls/3323/comments","review_comment_url":"https://api.github.com/repos/facebook/presto/pulls/comments{/number}","comments_url":"https://api.github.com/repos/facebook/presto/issues/3323/comments","statuses_url":"https://api.github.com/repos/facebook/presto/statuses/6a51258c484b61c97b2e2adce72b954ca3a4124c","head":{"label":"nileema:window-optimizer-fix","ref":"window-optimizer-fix","sha":"6a51258c484b61c97b2e2adce72b954ca3a4124c","user":{"login":"nileema","id":1034143,"avatar_url":"https://avatars.githubusercontent.com/u/1034143?v=3","gravatar_id":"","url":"https://api.github.com/users/nileema","html_url":"https://github.com/nileema","followers_url":"https://api.github.com/users/nileema/followers","following_url":"https://api.github.com/users/nileema/following{/other_user}","gists_url":"https://api.github.com/users/nileema/gists{/gist_id}","starred_url":"https://api.github.com/users/nileema/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nileema/subscriptions","organizations_url":"https://api.github.com/users/nileema/orgs","repos_url":"https://api.github.com/users/nileema/repos","events_url":"https://api.github.com/users/nileema/events{/privacy}","received_events_url":"https://api.github.com/users/nileema/received_events","type":"User","site_admin":false},"repo":{"id":12574341,"name":"presto","full_name":"nileema/presto","owner":{"login":"nileema","id":1034143,"avatar_url":"https://avatars.githubusercontent.com/u/1034143?v=3","gravatar_id":"","url":"https://api.github.com/users/nileema","html_url":"https://github.com/nileema","followers_url":"https://api.github.com/users/nileema/followers","following_url":"https://api.github.com/users/nileema/following{/other_user}","gists_url":"https://api.github.com/users/nileema/gists{/gist_id}","starred_url":"https://api.github.com/users/nileema/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nileema/subscriptions","organizations_url":"https://api.github.com/users/nileema/orgs","repos_url":"https://api.github.com/users/nileema/repos","events_url":"https://api.github.com/users/nileema/events{/privacy}","received_events_url":"https://api.github.com/users/nileema/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/nileema/presto","description":"","fork":true,"url":"https://api.github.com/repos/nileema/presto","forks_url":"https://api.github.com/repos/nileema/presto/forks","keys_url":"https://api.github.com/repos/nileema/presto/keys{/key_id}","collaborators_url":"https://api.github.com/repos/nileema/presto/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/nileema/presto/teams","hooks_url":"https://api.github.com/repos/nileema/presto/hooks","issue_events_url":"https://api.github.com/repos/nileema/presto/issues/events{/number}","events_url":"https://api.github.com/repos/nileema/presto/events","assignees_url":"https://api.github.com/repos/nileema/presto/assignees{/user}","branches_url":"https://api.github.com/repos/nileema/presto/branches{/branch}","tags_url":"https://api.github.com/repos/nileema/presto/tags","blobs_url":"https://api.github.com/repos/nileema/presto/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/nileema/presto/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/nileema/presto/git/refs{/sha}","trees_url":"https://api.github.com/repos/nileema/presto/git/trees{/sha}","statuses_url":"https://api.github.com/repos/nileema/presto/statuses/{sha}","languages_url":"https://api.github.com/repos/nileema/presto/languages","stargazers_url":"https://api.github.com/repos/nileema/presto/stargazers","contributors_url":"https://api.github.com/repos/nileema/presto/contributors","subscribers_url":"https://api.github.com/repos/nileema/presto/subscribers","subscription_url":"https://api.github.com/repos/nileema/presto/subscription","commits_url":"https://api.github.com/repos/nileema/presto/commits{/sha}","git_commits_url":"https://api.github.com/repos/nileema/presto/git/commits{/sha}","comments_url":"https://api.github.com/repos/nileema/presto/comments{/number}","issue_comment_url":"https://api.github.com/repos/nileema/presto/issues/comments{/number}","contents_url":"https://api.github.com/repos/nileema/presto/contents/{+path}","compare_url":"https://api.github.com/repos/nileema/presto/compare/{base}...{head}","merges_url":"https://api.github.com/repos/nileema/presto/merges","archive_url":"https://api.github.com/repos/nileema/presto/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/nileema/presto/downloads","issues_url":"https://api.github.com/repos/nileema/presto/issues{/number}","pulls_url":"https://api.github.com/repos/nileema/presto/pulls{/number}","milestones_url":"https://api.github.com/repos/nileema/presto/milestones{/number}","notifications_url":"https://api.github.com/repos/nileema/presto/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/nileema/presto/labels{/name}","releases_url":"https://api.github.com/repos/nileema/presto/releases{/id}","created_at":"2013-09-03T20:40:19Z","updated_at":"2014-08-13T18:36:54Z","pushed_at":"2015-07-22T01:22:43Z","git_url":"git://github.com/nileema/presto.git","ssh_url":"git@github.com:nileema/presto.git","clone_url":"https://github.com/nileema/presto.git","svn_url":"https://github.com/nileema/presto","homepage":null,"size":40861,"stargazers_count":0,"watchers_count":0,"language":"Java","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":"master"}},"base":{"label":"facebook:master","ref":"master","sha":"daa77a041d54c590c7372460999f7a9368c323da","user":{"login":"facebook","id":69631,"avatar_url":"https://avatars.githubusercontent.com/u/69631?v=3","gravatar_id":"","url":"https://api.github.com/users/facebook","html_url":"https://github.com/facebook","followers_url":"https://api.github.com/users/facebook/followers","following_url":"https://api.github.com/users/facebook/following{/other_user}","gists_url":"https://api.github.com/users/facebook/gists{/gist_id}","starred_url":"https://api.github.com/users/facebook/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/facebook/subscriptions","organizations_url":"https://api.github.com/users/facebook/orgs","repos_url":"https://api.github.com/users/facebook/repos","events_url":"https://api.github.com/users/facebook/events{/privacy}","received_events_url":"https://api.github.com/users/facebook/received_events","type":"Organization","site_admin":false},"repo":{"id":5349565,"name":"presto","full_name":"facebook/presto","owner":{"login":"facebook","id":69631,"avatar_url":"https://avatars.githubusercontent.com/u/69631?v=3","gravatar_id":"","url":"https://api.github.com/users/facebook","html_url":"https://github.com/facebook","followers_url":"https://api.github.com/users/facebook/followers","following_url":"https://api.github.com/users/facebook/following{/other_user}","gists_url":"https://api.github.com/users/facebook/gists{/gist_id}","starred_url":"https://api.github.com/users/facebook/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/facebook/subscriptions","organizations_url":"https://api.github.com/users/facebook/orgs","repos_url":"https://api.github.com/users/facebook/repos","events_url":"https://api.github.com/users/facebook/events{/privacy}","received_events_url":"https://api.github.com/users/facebook/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/facebook/presto","description":"Distributed SQL query engine for running interactive analytic queries against big data sources.","fork":false,"url":"https://api.github.com/repos/facebook/presto","forks_url":"https://api.github.com/repos/facebook/presto/forks","keys_url":"https://api.github.com/repos/facebook/presto/keys{/key_id}","collaborators_url":"https://api.github.com/repos/facebook/presto/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/facebook/presto/teams","hooks_url":"https://api.github.com/repos/facebook/presto/hooks","issue_events_url":"https://api.github.com/repos/facebook/presto/issues/events{/number}","events_url":"https://api.github.com/repos/facebook/presto/events","assignees_url":"https://api.github.com/repos/facebook/presto/assignees{/user}","branches_url":"https://api.github.com/repos/facebook/presto/branches{/branch}","tags_url":"https://api.github.com/repos/facebook/presto/tags","blobs_url":"https://api.github.com/repos/facebook/presto/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/facebook/presto/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/facebook/presto/git/refs{/sha}","trees_url":"https://api.github.com/repos/facebook/presto/git/trees{/sha}","statuses_url":"https://api.github.com/repos/facebook/presto/statuses/{sha}","languages_url":"https://api.github.com/repos/facebook/presto/languages","stargazers_url":"https://api.github.com/repos/facebook/presto/stargazers","contributors_url":"https://api.github.com/repos/facebook/presto/contributors","subscribers_url":"https://api.github.com/repos/facebook/presto/subscribers","subscription_url":"https://api.github.com/repos/facebook/presto/subscription","commits_url":"https://api.github.com/repos/facebook/presto/commits{/sha}","git_commits_url":"https://api.github.com/repos/facebook/presto/git/commits{/sha}","comments_url":"https://api.github.com/repos/facebook/presto/comments{/number}","issue_comment_url":"https://api.github.com/repos/facebook/presto/issues/comments{/number}","contents_url":"https://api.github.com/repos/facebook/presto/contents/{+path}","compare_url":"https://api.github.com/repos/facebook/presto/compare/{base}...{head}","merges_url":"https://api.github.com/repos/facebook/presto/merges","archive_url":"https://api.github.com/repos/facebook/presto/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/facebook/presto/downloads","issues_url":"https://api.github.com/repos/facebook/presto/issues{/number}","pulls_url":"https://api.github.com/repos/facebook/presto/pulls{/number}","milestones_url":"https://api.github.com/repos/facebook/presto/milestones{/number}","notifications_url":"https://api.github.com/repos/facebook/presto/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/facebook/presto/labels{/name}","releases_url":"https://api.github.com/repos/facebook/presto/releases{/id}","created_at":"2012-08-09T01:03:37Z","updated_at":"2015-07-22T01:48:18Z","pushed_at":"2015-07-22T01:50:11Z","git_url":"git://github.com/facebook/presto.git","ssh_url":"git@github.com:facebook/presto.git","clone_url":"https://github.com/facebook/presto.git","svn_url":"https://github.com/facebook/presto","homepage":"https://prestodb.io","size":93443,"stargazers_count":3642,"watchers_count":3642,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":915,"mirror_url":null,"open_issues_count":309,"forks":915,"open_issues":309,"watchers":3642,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/facebook/presto/pulls/3323"},"html":{"href":"https://github.com/facebook/presto/pull/3323"},"issue":{"href":"https://api.github.com/repos/facebook/presto/issues/3323"},"comments":{"href":"https://api.github.com/repos/facebook/presto/issues/3323/comments"},"review_comments":{"href":"https://api.github.com/repos/facebook/presto/pulls/3323/comments"},"review_comment":{"href":"https://api.github.com/repos/facebook/presto/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/facebook/presto/pulls/3323/commits"},"statuses":{"href":"https://api.github.com/repos/facebook/presto/statuses/6a51258c484b61c97b2e2adce72b954ca3a4124c"}}}}