{"action":"created","comment":{"url":"https://api.github.com/repos/prestodb/presto/pulls/comments/177574116","pull_request_review_id":107462157,"id":177574116,"diff_hunk":"@@ -190,7 +176,19 @@ public HivePartitionResult getPartitions(SemiTransactionalHiveMetastore metastor\n         return TupleDomain.withColumnDomains(builder.build());\n     }\n \n-    private Optional<Map<ColumnHandle, NullableValue>> parseValuesAndFilterPartition(String partitionName, List<HiveColumnHandle> partitionColumns, List<Type> partitionTypes, Constraint<ColumnHandle> constraint)\n+    private static class PartitionNameAndValues","path":"presto-hive/src/main/java/com/facebook/presto/hive/HivePartitionManager.java","position":96,"original_position":96,"commit_id":"02e52bb877992fbed7d47d2be37558e5403c3318","original_commit_id":"0a97981a50d66383baec73b7d1eae8d519cc1e77","user":{"login":"electrum","id":9230,"avatar_url":"https://avatars1.githubusercontent.com/u/9230?v=4","gravatar_id":"","url":"https://api.github.com/users/electrum","html_url":"https://github.com/electrum","followers_url":"https://api.github.com/users/electrum/followers","following_url":"https://api.github.com/users/electrum/following{/other_user}","gists_url":"https://api.github.com/users/electrum/gists{/gist_id}","starred_url":"https://api.github.com/users/electrum/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/electrum/subscriptions","organizations_url":"https://api.github.com/users/electrum/orgs","repos_url":"https://api.github.com/users/electrum/repos","events_url":"https://api.github.com/users/electrum/events{/privacy}","received_events_url":"https://api.github.com/users/electrum/received_events","type":"User","site_admin":false},"body":"I'm not sure we need this class. We could change `parseValuesAndFilterPartition` to return `Optional<HivePartition>`. Would require passing a couple extra arguments but the code would be simpler.","created_at":"2018-03-27T21:12:09Z","updated_at":"2018-03-27T21:42:34Z","html_url":"https://github.com/prestodb/presto/pull/10259#discussion_r177574116","pull_request_url":"https://api.github.com/repos/prestodb/presto/pulls/10259","author_association":"OWNER","_links":{"self":{"href":"https://api.github.com/repos/prestodb/presto/pulls/comments/177574116"},"html":{"href":"https://github.com/prestodb/presto/pull/10259#discussion_r177574116"},"pull_request":{"href":"https://api.github.com/repos/prestodb/presto/pulls/10259"}}},"pull_request":{"url":"https://api.github.com/repos/prestodb/presto/pulls/10259","id":177502710,"html_url":"https://github.com/prestodb/presto/pull/10259","diff_url":"https://github.com/prestodb/presto/pull/10259.diff","patch_url":"https://github.com/prestodb/presto/pull/10259.patch","issue_url":"https://api.github.com/repos/prestodb/presto/issues/10259","number":10259,"state":"open","locked":false,"title":"Expose \"$partitions\" system tables for partitioned Hive tables","user":{"login":"losipiuk","id":890111,"avatar_url":"https://avatars1.githubusercontent.com/u/890111?v=4","gravatar_id":"","url":"https://api.github.com/users/losipiuk","html_url":"https://github.com/losipiuk","followers_url":"https://api.github.com/users/losipiuk/followers","following_url":"https://api.github.com/users/losipiuk/following{/other_user}","gists_url":"https://api.github.com/users/losipiuk/gists{/gist_id}","starred_url":"https://api.github.com/users/losipiuk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/losipiuk/subscriptions","organizations_url":"https://api.github.com/users/losipiuk/orgs","repos_url":"https://api.github.com/users/losipiuk/repos","events_url":"https://api.github.com/users/losipiuk/events{/privacy}","received_events_url":"https://api.github.com/users/losipiuk/received_events","type":"User","site_admin":false},"body":"@electrum, @findepi \r\n**Please just do high level review to validate if approach is generally OK**\r\n\r\nThis is an try to workaround the problem with listing Hive partitions when there are more of them than configuration set hard limit.\r\nWe decided that want to try approach when for each partitioned Hive table we expose a counterpart system table (suffixed with `$partitions`) which \r\nwill expose all the partitioning keys.\r\n\r\nThis PR achieves that by exposing new `Optional<SystemTable> ConnectorMetadata.getSystemTable()` method.\r\nThe method is then plugged to per-connector SystemMemory connector via `SystemTablesProvider` (which used to be just `List<SystemTable>` before).\r\nThe `SystemTablesProvider` is responsible for listing and resolving system tables.\r\nThe used implementation is provided with `List<SystemTable>` (for sake of static system tables) and `Metadata` which is used to delegate calls to `ConnectorMetadata.getSystemTable` for dynamic ones.\r\n\r\nThe approach generally seems to work but please let me know what you think about it.\r\n\r\nOne limitation I see with current implementation is that we are only listing the static system tables. This leads to some weirdnesses. E.g you will get empty table when query `SHOW COLUMNS FROM \"table$partitions\"`, because it is rewritten to `information_schema` query which depends on fact that queried tables are listed via `ConnectorMedata.listTables`. \r\nPotentially this can be extended by adding `ConnectorMetadata.listSystemTables` method. Yet for Hive we cannot cheaply test if table is partitioned or not (I think).\r\nAnd without that check it gets problematic if the `$partitions` table is queried for table which is not actually partitioned as we do not have any columns to return. But generally it should be doable - we just need to think about what is exact expected behaviour.\r\n\r\nEdit: addressed limitation with `Use SystemTablesProvider.getSystemTable in listTableColumns` commit.\r\n","created_at":"2018-03-26T16:37:32Z","updated_at":"2018-03-27T21:42:34Z","closed_at":null,"merged_at":null,"merge_commit_sha":"32ab300645a9c1c9c5b08baaaf8ec377501126f8","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":196992743,"url":"https://api.github.com/repos/prestodb/presto/labels/CLA%20Signed","name":"CLA Signed","color":"ededed","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/prestodb/presto/pulls/10259/commits","review_comments_url":"https://api.github.com/repos/prestodb/presto/pulls/10259/comments","review_comment_url":"https://api.github.com/repos/prestodb/presto/pulls/comments{/number}","comments_url":"https://api.github.com/repos/prestodb/presto/issues/10259/comments","statuses_url":"https://api.github.com/repos/prestodb/presto/statuses/02e52bb877992fbed7d47d2be37558e5403c3318","head":{"label":"starburstdata:lo/partitions_table2","ref":"lo/partitions_table2","sha":"02e52bb877992fbed7d47d2be37558e5403c3318","user":{"login":"starburstdata","id":33209422,"avatar_url":"https://avatars3.githubusercontent.com/u/33209422?v=4","gravatar_id":"","url":"https://api.github.com/users/starburstdata","html_url":"https://github.com/starburstdata","followers_url":"https://api.github.com/users/starburstdata/followers","following_url":"https://api.github.com/users/starburstdata/following{/other_user}","gists_url":"https://api.github.com/users/starburstdata/gists{/gist_id}","starred_url":"https://api.github.com/users/starburstdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/starburstdata/subscriptions","organizations_url":"https://api.github.com/users/starburstdata/orgs","repos_url":"https://api.github.com/users/starburstdata/repos","events_url":"https://api.github.com/users/starburstdata/events{/privacy}","received_events_url":"https://api.github.com/users/starburstdata/received_events","type":"Organization","site_admin":false},"repo":{"id":108882128,"name":"presto","full_name":"starburstdata/presto","owner":{"login":"starburstdata","id":33209422,"avatar_url":"https://avatars3.githubusercontent.com/u/33209422?v=4","gravatar_id":"","url":"https://api.github.com/users/starburstdata","html_url":"https://github.com/starburstdata","followers_url":"https://api.github.com/users/starburstdata/followers","following_url":"https://api.github.com/users/starburstdata/following{/other_user}","gists_url":"https://api.github.com/users/starburstdata/gists{/gist_id}","starred_url":"https://api.github.com/users/starburstdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/starburstdata/subscriptions","organizations_url":"https://api.github.com/users/starburstdata/orgs","repos_url":"https://api.github.com/users/starburstdata/repos","events_url":"https://api.github.com/users/starburstdata/events{/privacy}","received_events_url":"https://api.github.com/users/starburstdata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/starburstdata/presto","description":"Starburst Enterprise Distribution of Presto","fork":true,"url":"https://api.github.com/repos/starburstdata/presto","forks_url":"https://api.github.com/repos/starburstdata/presto/forks","keys_url":"https://api.github.com/repos/starburstdata/presto/keys{/key_id}","collaborators_url":"https://api.github.com/repos/starburstdata/presto/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/starburstdata/presto/teams","hooks_url":"https://api.github.com/repos/starburstdata/presto/hooks","issue_events_url":"https://api.github.com/repos/starburstdata/presto/issues/events{/number}","events_url":"https://api.github.com/repos/starburstdata/presto/events","assignees_url":"https://api.github.com/repos/starburstdata/presto/assignees{/user}","branches_url":"https://api.github.com/repos/starburstdata/presto/branches{/branch}","tags_url":"https://api.github.com/repos/starburstdata/presto/tags","blobs_url":"https://api.github.com/repos/starburstdata/presto/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/starburstdata/presto/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/starburstdata/presto/git/refs{/sha}","trees_url":"https://api.github.com/repos/starburstdata/presto/git/trees{/sha}","statuses_url":"https://api.github.com/repos/starburstdata/presto/statuses/{sha}","languages_url":"https://api.github.com/repos/starburstdata/presto/languages","stargazers_url":"https://api.github.com/repos/starburstdata/presto/stargazers","contributors_url":"https://api.github.com/repos/starburstdata/presto/contributors","subscribers_url":"https://api.github.com/repos/starburstdata/presto/subscribers","subscription_url":"https://api.github.com/repos/starburstdata/presto/subscription","commits_url":"https://api.github.com/repos/starburstdata/presto/commits{/sha}","git_commits_url":"https://api.github.com/repos/starburstdata/presto/git/commits{/sha}","comments_url":"https://api.github.com/repos/starburstdata/presto/comments{/number}","issue_comment_url":"https://api.github.com/repos/starburstdata/presto/issues/comments{/number}","contents_url":"https://api.github.com/repos/starburstdata/presto/contents/{+path}","compare_url":"https://api.github.com/repos/starburstdata/presto/compare/{base}...{head}","merges_url":"https://api.github.com/repos/starburstdata/presto/merges","archive_url":"https://api.github.com/repos/starburstdata/presto/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/starburstdata/presto/downloads","issues_url":"https://api.github.com/repos/starburstdata/presto/issues{/number}","pulls_url":"https://api.github.com/repos/starburstdata/presto/pulls{/number}","milestones_url":"https://api.github.com/repos/starburstdata/presto/milestones{/number}","notifications_url":"https://api.github.com/repos/starburstdata/presto/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/starburstdata/presto/labels{/name}","releases_url":"https://api.github.com/repos/starburstdata/presto/releases{/id}","deployments_url":"https://api.github.com/repos/starburstdata/presto/deployments","created_at":"2017-10-30T17:05:09Z","updated_at":"2018-03-27T20:12:16Z","pushed_at":"2018-03-27T20:12:15Z","git_url":"git://github.com/starburstdata/presto.git","ssh_url":"git@github.com:starburstdata/presto.git","clone_url":"https://github.com/starburstdata/presto.git","svn_url":"https://github.com/starburstdata/presto","homepage":"https://www.starburstdata.com","size":93593,"stargazers_count":17,"watchers_count":17,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":7,"license":null,"forks":0,"open_issues":7,"watchers":17,"default_branch":"master"}},"base":{"label":"prestodb:master","ref":"master","sha":"5d6072ed899117447c21654a7ba5ded5c2687932","user":{"login":"prestodb","id":6882181,"avatar_url":"https://avatars0.githubusercontent.com/u/6882181?v=4","gravatar_id":"","url":"https://api.github.com/users/prestodb","html_url":"https://github.com/prestodb","followers_url":"https://api.github.com/users/prestodb/followers","following_url":"https://api.github.com/users/prestodb/following{/other_user}","gists_url":"https://api.github.com/users/prestodb/gists{/gist_id}","starred_url":"https://api.github.com/users/prestodb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/prestodb/subscriptions","organizations_url":"https://api.github.com/users/prestodb/orgs","repos_url":"https://api.github.com/users/prestodb/repos","events_url":"https://api.github.com/users/prestodb/events{/privacy}","received_events_url":"https://api.github.com/users/prestodb/received_events","type":"Organization","site_admin":false},"repo":{"id":5349565,"name":"presto","full_name":"prestodb/presto","owner":{"login":"prestodb","id":6882181,"avatar_url":"https://avatars0.githubusercontent.com/u/6882181?v=4","gravatar_id":"","url":"https://api.github.com/users/prestodb","html_url":"https://github.com/prestodb","followers_url":"https://api.github.com/users/prestodb/followers","following_url":"https://api.github.com/users/prestodb/following{/other_user}","gists_url":"https://api.github.com/users/prestodb/gists{/gist_id}","starred_url":"https://api.github.com/users/prestodb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/prestodb/subscriptions","organizations_url":"https://api.github.com/users/prestodb/orgs","repos_url":"https://api.github.com/users/prestodb/repos","events_url":"https://api.github.com/users/prestodb/events{/privacy}","received_events_url":"https://api.github.com/users/prestodb/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/prestodb/presto","description":"Distributed SQL query engine for big data","fork":false,"url":"https://api.github.com/repos/prestodb/presto","forks_url":"https://api.github.com/repos/prestodb/presto/forks","keys_url":"https://api.github.com/repos/prestodb/presto/keys{/key_id}","collaborators_url":"https://api.github.com/repos/prestodb/presto/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/prestodb/presto/teams","hooks_url":"https://api.github.com/repos/prestodb/presto/hooks","issue_events_url":"https://api.github.com/repos/prestodb/presto/issues/events{/number}","events_url":"https://api.github.com/repos/prestodb/presto/events","assignees_url":"https://api.github.com/repos/prestodb/presto/assignees{/user}","branches_url":"https://api.github.com/repos/prestodb/presto/branches{/branch}","tags_url":"https://api.github.com/repos/prestodb/presto/tags","blobs_url":"https://api.github.com/repos/prestodb/presto/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/prestodb/presto/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/prestodb/presto/git/refs{/sha}","trees_url":"https://api.github.com/repos/prestodb/presto/git/trees{/sha}","statuses_url":"https://api.github.com/repos/prestodb/presto/statuses/{sha}","languages_url":"https://api.github.com/repos/prestodb/presto/languages","stargazers_url":"https://api.github.com/repos/prestodb/presto/stargazers","contributors_url":"https://api.github.com/repos/prestodb/presto/contributors","subscribers_url":"https://api.github.com/repos/prestodb/presto/subscribers","subscription_url":"https://api.github.com/repos/prestodb/presto/subscription","commits_url":"https://api.github.com/repos/prestodb/presto/commits{/sha}","git_commits_url":"https://api.github.com/repos/prestodb/presto/git/commits{/sha}","comments_url":"https://api.github.com/repos/prestodb/presto/comments{/number}","issue_comment_url":"https://api.github.com/repos/prestodb/presto/issues/comments{/number}","contents_url":"https://api.github.com/repos/prestodb/presto/contents/{+path}","compare_url":"https://api.github.com/repos/prestodb/presto/compare/{base}...{head}","merges_url":"https://api.github.com/repos/prestodb/presto/merges","archive_url":"https://api.github.com/repos/prestodb/presto/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/prestodb/presto/downloads","issues_url":"https://api.github.com/repos/prestodb/presto/issues{/number}","pulls_url":"https://api.github.com/repos/prestodb/presto/pulls{/number}","milestones_url":"https://api.github.com/repos/prestodb/presto/milestones{/number}","notifications_url":"https://api.github.com/repos/prestodb/presto/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/prestodb/presto/labels{/name}","releases_url":"https://api.github.com/repos/prestodb/presto/releases{/id}","deployments_url":"https://api.github.com/repos/prestodb/presto/deployments","created_at":"2012-08-09T01:03:37Z","updated_at":"2018-03-27T20:05:14Z","pushed_at":"2018-03-27T20:32:43Z","git_url":"git://github.com/prestodb/presto.git","ssh_url":"git@github.com:prestodb/presto.git","clone_url":"https://github.com/prestodb/presto.git","svn_url":"https://github.com/prestodb/presto","homepage":"https://prestodb.io","size":88936,"stargazers_count":7268,"watchers_count":7268,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2443,"mirror_url":null,"archived":false,"open_issues_count":1252,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":2443,"open_issues":1252,"watchers":7268,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/prestodb/presto/pulls/10259"},"html":{"href":"https://github.com/prestodb/presto/pull/10259"},"issue":{"href":"https://api.github.com/repos/prestodb/presto/issues/10259"},"comments":{"href":"https://api.github.com/repos/prestodb/presto/issues/10259/comments"},"review_comments":{"href":"https://api.github.com/repos/prestodb/presto/pulls/10259/comments"},"review_comment":{"href":"https://api.github.com/repos/prestodb/presto/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/prestodb/presto/pulls/10259/commits"},"statuses":{"href":"https://api.github.com/repos/prestodb/presto/statuses/02e52bb877992fbed7d47d2be37558e5403c3318"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"electrum"}}