payload: {"action":"created","comment":{"url":"https://api.github.com/repos/apache/incubator-pinot/pulls/comments/277828468","pull_request_review_id":229741415,"id":277828468,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDI3NzgyODQ2OA==","diff_hunk":"@@ -48,77 +48,84 @@ public HelixExternalViewBasedTimeBoundaryService(ZkHelixPropertyStore<ZNRecord>\n   }\n \n   public void updateTimeBoundaryService(ExternalView externalView) {\n-    if (_propertyStore == null) {\n-      return;\n-    }\n-    String tableName = externalView.getResourceName();\n-    // Do nothing for realtime table.\n-    if (TableNameBuilder.getTableTypeFromTableName(tableName) == TableType.REALTIME) {\n+    String tableNameWithType = externalView.getResourceName();\n+\n+    // Skip real-time table, only use offline table to update the time boundary\n+    if (TableNameBuilder.getTableTypeFromTableName(tableNameWithType) == TableType.REALTIME) {\n       return;\n     }\n \n     Set<String> offlineSegmentsServing = externalView.getPartitionSet();\n     if (offlineSegmentsServing.isEmpty()) {\n-      LOGGER.info(\"Skipping updating time boundary service for table '{}' with no offline segments.\", tableName);\n+      LOGGER.warn(\"Skip updating time boundary for table: '{}' with no offline segment\", tableNameWithType);\n       return;\n     }\n \n-    TableConfig offlineTableConfig = ZKMetadataProvider.getOfflineTableConfig(_propertyStore, tableName);\n-    assert offlineTableConfig != null;\n-    TimeUnit tableTimeUnit = offlineTableConfig.getValidationConfig().getTimeType();\n-    if (tableTimeUnit == null) {\n-      LOGGER.info(\"Skipping updating time boundary service for table '{}' because time unit is not set\", tableName);\n+    Schema schema = ZKMetadataProvider.getTableSchema(_propertyStore, tableNameWithType);\n+    assert schema != null;\n+    // TODO: support SDF\n+    String timeColumn = schema.getTimeColumnName();","path":"pinot-broker/src/main/java/org/apache/pinot/broker/routing/HelixExternalViewBasedTimeBoundaryService.java","position":44,"original_position":42,"commit_id":"2bd709ee61ed17d14aef3dadf325db36778e2118","original_commit_id":"8ad3619627b908eee160992c61d483d100442ff9","user":{"login":"Jackie-Jiang","id":17555551,"node_id":"MDQ6VXNlcjE3NTU1NTUx","avatar_url":"https://avatars3.githubusercontent.com/u/17555551?v=4","gravatar_id":"","url":"https://api.github.com/users/Jackie-Jiang","html_url":"https://github.com/Jackie-Jiang","followers_url":"https://api.github.com/users/Jackie-Jiang/followers","following_url":"https://api.github.com/users/Jackie-Jiang/following{/other_user}","gists_url":"https://api.github.com/users/Jackie-Jiang/gists{/gist_id}","starred_url":"https://api.github.com/users/Jackie-Jiang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Jackie-Jiang/subscriptions","organizations_url":"https://api.github.com/users/Jackie-Jiang/orgs","repos_url":"https://api.github.com/users/Jackie-Jiang/repos","events_url":"https://api.github.com/users/Jackie-Jiang/events{/privacy}","received_events_url":"https://api.github.com/users/Jackie-Jiang/received_events","type":"User","site_admin":false},"body":"Discussed with @npawar. When we start using dateTime, pick the time column from table config, and use the DateTimeFieldSpec from the schema to determine the time unit. For now, added a TODO.","created_at":"2019-04-23T19:06:30Z","updated_at":"2019-04-23T19:06:30Z","html_url":"https://github.com/apache/incubator-pinot/pull/4156#discussion_r277828468","pull_request_url":"https://api.github.com/repos/apache/incubator-pinot/pulls/4156","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/apache/incubator-pinot/pulls/comments/277828468"},"html":{"href":"https://github.com/apache/incubator-pinot/pull/4156#discussion_r277828468"},"pull_request":{"href":"https://api.github.com/repos/apache/incubator-pinot/pulls/4156"}},"in_reply_to_id":277557033},"pull_request":{"url":"https://api.github.com/repos/apache/incubator-pinot/pulls/4156","id":272588591,"node_id":"MDExOlB1bGxSZXF1ZXN0MjcyNTg4NTkx","html_url":"https://github.com/apache/incubator-pinot/pull/4156","diff_url":"https://github.com/apache/incubator-pinot/pull/4156.diff","patch_url":"https://github.com/apache/incubator-pinot/pull/4156.patch","issue_url":"https://api.github.com/repos/apache/incubator-pinot/issues/4156","number":4156,"state":"open","locked":false,"title":"Refactor HelixExternalViewBasedTimeBoundaryService to support all time units","user":{"login":"Jackie-Jiang","id":17555551,"node_id":"MDQ6VXNlcjE3NTU1NTUx","avatar_url":"https://avatars3.githubusercontent.com/u/17555551?v=4","gravatar_id":"","url":"https://api.github.com/users/Jackie-Jiang","html_url":"https://github.com/Jackie-Jiang","followers_url":"https://api.github.com/users/Jackie-Jiang/followers","following_url":"https://api.github.com/users/Jackie-Jiang/following{/other_user}","gists_url":"https://api.github.com/users/Jackie-Jiang/gists{/gist_id}","starred_url":"https://api.github.com/users/Jackie-Jiang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Jackie-Jiang/subscriptions","organizations_url":"https://api.github.com/users/Jackie-Jiang/orgs","repos_url":"https://api.github.com/users/Jackie-Jiang/repos","events_url":"https://api.github.com/users/Jackie-Jiang/events{/privacy}","received_events_url":"https://api.github.com/users/Jackie-Jiang/received_events","type":"User","site_admin":false},"body":"Currently we pick the segment end time as the time boundary, and\nappend filter 'timeColumn < boundary' to offline table and filter\n'timeColumn >= boundary' to realtime table to achieve the hybrid\ntable federation. The problem with this is that, if the time unit\nis not DAYS (for example, MILLISECONDS), and the offline table has\nmultiple daily segments to push, then we might get incomplete\nresult before all offline segments are pushed.\n\nThe solution is: always use (end time - 1 DAY) as the time\nboundary, append filter 'timeColumn <= boundary' to offline table\nand 'timeColumn > boundary' to realtime table. This can ensure\nall daily pushed segments or hourly pushed segments be covered\nregardless of the time unit.\n\nAlso, we should use the time spec in schema as the source of truth\nfor time column because data is generated based on the schema. In\nthe future we might remove the timeColumnName and timeType fields\nfrom SegmentsValidationAndRetentionConfig.","created_at":"2019-04-23T05:33:33Z","updated_at":"2019-04-23T19:06:30Z","closed_at":null,"merged_at":null,"merge_commit_sha":"3de294b799bd4b5158ec04f4e25658a4749c5b94","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/apache/incubator-pinot/pulls/4156/commits","review_comments_url":"https://api.github.com/repos/apache/incubator-pinot/pulls/4156/comments","review_comment_url":"https://api.github.com/repos/apache/incubator-pinot/pulls/comments{/number}","comments_url":"https://api.github.com/repos/apache/incubator-pinot/issues/4156/comments","statuses_url":"https://api.github.com/repos/apache/incubator-pinot/statuses/2bd709ee61ed17d14aef3dadf325db36778e2118","head":{"label":"apache:time_boundary","ref":"time_boundary","sha":"2bd709ee61ed17d14aef3dadf325db36778e2118","user":{"login":"apache","id":47359,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ3MzU5","avatar_url":"https://avatars0.githubusercontent.com/u/47359?v=4","gravatar_id":"","url":"https://api.github.com/users/apache","html_url":"https://github.com/apache","followers_url":"https://api.github.com/users/apache/followers","following_url":"https://api.github.com/users/apache/following{/other_user}","gists_url":"https://api.github.com/users/apache/gists{/gist_id}","starred_url":"https://api.github.com/users/apache/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apache/subscriptions","organizations_url":"https://api.github.com/users/apache/orgs","repos_url":"https://api.github.com/users/apache/repos","events_url":"https://api.github.com/users/apache/events{/privacy}","received_events_url":"https://api.github.com/users/apache/received_events","type":"Organization","site_admin":false},"repo":{"id":19961085,"node_id":"MDEwOlJlcG9zaXRvcnkxOTk2MTA4NQ==","name":"incubator-pinot","full_name":"apache/incubator-pinot","private":false,"owner":{"login":"apache","id":47359,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ3MzU5","avatar_url":"https://avatars0.githubusercontent.com/u/47359?v=4","gravatar_id":"","url":"https://api.github.com/users/apache","html_url":"https://github.com/apache","followers_url":"https://api.github.com/users/apache/followers","following_url":"https://api.github.com/users/apache/following{/other_user}","gists_url":"https://api.github.com/users/apache/gists{/gist_id}","starred_url":"https://api.github.com/users/apache/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apache/subscriptions","organizations_url":"https://api.github.com/users/apache/orgs","repos_url":"https://api.github.com/users/apache/repos","events_url":"https://api.github.com/users/apache/events{/privacy}","received_events_url":"https://api.github.com/users/apache/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/apache/incubator-pinot","description":"Apache Pinot (Incubating) - A realtime distributed OLAP datastore","fork":false,"url":"https://api.github.com/repos/apache/incubator-pinot","forks_url":"https://api.github.com/repos/apache/incubator-pinot/forks","keys_url":"https://api.github.com/repos/apache/incubator-pinot/keys{/key_id}","collaborators_url":"https://api.github.com/repos/apache/incubator-pinot/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/apache/incubator-pinot/teams","hooks_url":"https://api.github.com/repos/apache/incubator-pinot/hooks","issue_events_url":"https://api.github.com/repos/apache/incubator-pinot/issues/events{/number}","events_url":"https://api.github.com/repos/apache/incubator-pinot/events","assignees_url":"https://api.github.com/repos/apache/incubator-pinot/assignees{/user}","branches_url":"https://api.github.com/repos/apache/incubator-pinot/branches{/branch}","tags_url":"https://api.github.com/repos/apache/incubator-pinot/tags","blobs_url":"https://api.github.com/repos/apache/incubator-pinot/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/apache/incubator-pinot/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/apache/incubator-pinot/git/refs{/sha}","trees_url":"https://api.github.com/repos/apache/incubator-pinot/git/trees{/sha}","statuses_url":"https://api.github.com/repos/apache/incubator-pinot/statuses/{sha}","languages_url":"https://api.github.com/repos/apache/incubator-pinot/languages","stargazers_url":"https://api.github.com/repos/apache/incubator-pinot/stargazers","contributors_url":"https://api.github.com/repos/apache/incubator-pinot/contributors","subscribers_url":"https://api.github.com/repos/apache/incubator-pinot/subscribers","subscription_url":"https://api.github.com/repos/apache/incubator-pinot/subscription","commits_url":"https://api.github.com/repos/apache/incubator-pinot/commits{/sha}","git_commits_url":"https://api.github.com/repos/apache/incubator-pinot/git/commits{/sha}","comments_url":"https://api.github.com/repos/apache/incubator-pinot/comments{/number}","issue_comment_url":"https://api.github.com/repos/apache/incubator-pinot/issues/comments{/number}","contents_url":"https://api.github.com/repos/apache/incubator-pinot/contents/{+path}","compare_url":"https://api.github.com/repos/apache/incubator-pinot/compare/{base}...{head}","merges_url":"https://api.github.com/repos/apache/incubator-pinot/merges","archive_url":"https://api.github.com/repos/apache/incubator-pinot/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/apache/incubator-pinot/downloads","issues_url":"https://api.github.com/repos/apache/incubator-pinot/issues{/number}","pulls_url":"https://api.github.com/repos/apache/incubator-pinot/pulls{/number}","milestones_url":"https://api.github.com/repos/apache/incubator-pinot/milestones{/number}","notifications_url":"https://api.github.com/repos/apache/incubator-pinot/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/apache/incubator-pinot/labels{/name}","releases_url":"https://api.github.com/repos/apache/incubator-pinot/releases{/id}","deployments_url":"https://api.github.com/repos/apache/incubator-pinot/deployments","created_at":"2014-05-19T23:27:48Z","updated_at":"2019-04-23T18:11:37Z","pushed_at":"2019-04-23T19:05:23Z","git_url":"git://github.com/apache/incubator-pinot.git","ssh_url":"git@github.com:apache/incubator-pinot.git","clone_url":"https://github.com/apache/incubator-pinot.git","svn_url":"https://github.com/apache/incubator-pinot","homepage":"https://pinot.apache.org","size":168495,"stargazers_count":2247,"watchers_count":2247,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":413,"mirror_url":null,"archived":false,"disabled":false,"open_issues_count":252,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":413,"open_issues":252,"watchers":2247,"default_branch":"master"}},"base":{"label":"apache:master","ref":"master","sha":"222162323d7a782a10d831ced93b95538a90e74a","user":{"login":"apache","id":47359,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ3MzU5","avatar_url":"https://avatars0.githubusercontent.com/u/47359?v=4","gravatar_id":"","url":"https://api.github.com/users/apache","html_url":"https://github.com/apache","followers_url":"https://api.github.com/users/apache/followers","following_url":"https://api.github.com/users/apache/following{/other_user}","gists_url":"https://api.github.com/users/apache/gists{/gist_id}","starred_url":"https://api.github.com/users/apache/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apache/subscriptions","organizations_url":"https://api.github.com/users/apache/orgs","repos_url":"https://api.github.com/users/apache/repos","events_url":"https://api.github.com/users/apache/events{/privacy}","received_events_url":"https://api.github.com/users/apache/received_events","type":"Organization","site_admin":false},"repo":{"id":19961085,"node_id":"MDEwOlJlcG9zaXRvcnkxOTk2MTA4NQ==","name":"incubator-pinot","full_name":"apache/incubator-pinot","private":false,"owner":{"login":"apache","id":47359,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ3MzU5","avatar_url":"https://avatars0.githubusercontent.com/u/47359?v=4","gravatar_id":"","url":"https://api.github.com/users/apache","html_url":"https://github.com/apache","followers_url":"https://api.github.com/users/apache/followers","following_url":"https://api.github.com/users/apache/following{/other_user}","gists_url":"https://api.github.com/users/apache/gists{/gist_id}","starred_url":"https://api.github.com/users/apache/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apache/subscriptions","organizations_url":"https://api.github.com/users/apache/orgs","repos_url":"https://api.github.com/users/apache/repos","events_url":"https://api.github.com/users/apache/events{/privacy}","received_events_url":"https://api.github.com/users/apache/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/apache/incubator-pinot","description":"Apache Pinot (Incubating) - A realtime distributed OLAP datastore","fork":false,"url":"https://api.github.com/repos/apache/incubator-pinot","forks_url":"https://api.github.com/repos/apache/incubator-pinot/forks","keys_url":"https://api.github.com/repos/apache/incubator-pinot/keys{/key_id}","collaborators_url":"https://api.github.com/repos/apache/incubator-pinot/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/apache/incubator-pinot/teams","hooks_url":"https://api.github.com/repos/apache/incubator-pinot/hooks","issue_events_url":"https://api.github.com/repos/apache/incubator-pinot/issues/events{/number}","events_url":"https://api.github.com/repos/apache/incubator-pinot/events","assignees_url":"https://api.github.com/repos/apache/incubator-pinot/assignees{/user}","branches_url":"https://api.github.com/repos/apache/incubator-pinot/branches{/branch}","tags_url":"https://api.github.com/repos/apache/incubator-pinot/tags","blobs_url":"https://api.github.com/repos/apache/incubator-pinot/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/apache/incubator-pinot/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/apache/incubator-pinot/git/refs{/sha}","trees_url":"https://api.github.com/repos/apache/incubator-pinot/git/trees{/sha}","statuses_url":"https://api.github.com/repos/apache/incubator-pinot/statuses/{sha}","languages_url":"https://api.github.com/repos/apache/incubator-pinot/languages","stargazers_url":"https://api.github.com/repos/apache/incubator-pinot/stargazers","contributors_url":"https://api.github.com/repos/apache/incubator-pinot/contributors","subscribers_url":"https://api.github.com/repos/apache/incubator-pinot/subscribers","subscription_url":"https://api.github.com/repos/apache/incubator-pinot/subscription","commits_url":"https://api.github.com/repos/apache/incubator-pinot/commits{/sha}","git_commits_url":"https://api.github.com/repos/apache/incubator-pinot/git/commits{/sha}","comments_url":"https://api.github.com/repos/apache/incubator-pinot/comments{/number}","issue_comment_url":"https://api.github.com/repos/apache/incubator-pinot/issues/comments{/number}","contents_url":"https://api.github.com/repos/apache/incubator-pinot/contents/{+path}","compare_url":"https://api.github.com/repos/apache/incubator-pinot/compare/{base}...{head}","merges_url":"https://api.github.com/repos/apache/incubator-pinot/merges","archive_url":"https://api.github.com/repos/apache/incubator-pinot/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/apache/incubator-pinot/downloads","issues_url":"https://api.github.com/repos/apache/incubator-pinot/issues{/number}","pulls_url":"https://api.github.com/repos/apache/incubator-pinot/pulls{/number}","milestones_url":"https://api.github.com/repos/apache/incubator-pinot/milestones{/number}","notifications_url":"https://api.github.com/repos/apache/incubator-pinot/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/apache/incubator-pinot/labels{/name}","releases_url":"https://api.github.com/repos/apache/incubator-pinot/releases{/id}","deployments_url":"https://api.github.com/repos/apache/incubator-pinot/deployments","created_at":"2014-05-19T23:27:48Z","updated_at":"2019-04-23T18:11:37Z","pushed_at":"2019-04-23T19:05:23Z","git_url":"git://github.com/apache/incubator-pinot.git","ssh_url":"git@github.com:apache/incubator-pinot.git","clone_url":"https://github.com/apache/incubator-pinot.git","svn_url":"https://github.com/apache/incubator-pinot","homepage":"https://pinot.apache.org","size":168495,"stargazers_count":2247,"watchers_count":2247,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":413,"mirror_url":null,"archived":false,"disabled":false,"open_issues_count":252,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":413,"open_issues":252,"watchers":2247,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/apache/incubator-pinot/pulls/4156"},"html":{"href":"https://github.com/apache/incubator-pinot/pull/4156"},"issue":{"href":"https://api.github.com/repos/apache/incubator-pinot/issues/4156"},"comments":{"href":"https://api.github.com/repos/apache/incubator-pinot/issues/4156/comments"},"review_comments":{"href":"https://api.github.com/repos/apache/incubator-pinot/pulls/4156/comments"},"review_comment":{"href":"https://api.github.com/repos/apache/incubator-pinot/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/apache/incubator-pinot/pulls/4156/commits"},"statuses":{"href":"https://api.github.com/repos/apache/incubator-pinot/statuses/2bd709ee61ed17d14aef3dadf325db36778e2118"}},"author_association":"CONTRIBUTOR"}}
other: {"actor":{"display_login":"Jackie-Jiang"}}