{"action":"created","comment":{"url":"https://api.github.com/repos/confluentinc/ksql/pulls/comments/223816328","pull_request_review_id":163015166,"id":223816328,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIyMzgxNjMyOA==","diff_hunk":"@@ -116,4 +119,9 @@ public KTable applyAggregate(\n             .advanceBy(advanceByUnit.toMillis(advanceBy))\n     ).aggregate(initializer, aggregator, materialized);\n   }\n+\n+  @Override\n+  public <K> Serde<Windowed<K>> getKeySerde(final Class<K> innerType) {\n+    return WindowedSerdes.timeWindowedSerdeFrom(innerType);\n+  }","path":"ksql-parser/src/main/java/io/confluent/ksql/parser/tree/HoppingWindowExpression.java","position":23,"original_position":23,"commit_id":"477b551e9aaec85d4414ac3fc339cf1515a4084e","original_commit_id":"477b551e9aaec85d4414ac3fc339cf1515a4084e","user":{"login":"hjafarpour","id":16784742,"node_id":"MDQ6VXNlcjE2Nzg0NzQy","avatar_url":"https://avatars3.githubusercontent.com/u/16784742?v=4","gravatar_id":"","url":"https://api.github.com/users/hjafarpour","html_url":"https://github.com/hjafarpour","followers_url":"https://api.github.com/users/hjafarpour/followers","following_url":"https://api.github.com/users/hjafarpour/following{/other_user}","gists_url":"https://api.github.com/users/hjafarpour/gists{/gist_id}","starred_url":"https://api.github.com/users/hjafarpour/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hjafarpour/subscriptions","organizations_url":"https://api.github.com/users/hjafarpour/orgs","repos_url":"https://api.github.com/users/hjafarpour/repos","events_url":"https://api.github.com/users/hjafarpour/events{/privacy}","received_events_url":"https://api.github.com/users/hjafarpour/received_events","type":"User","site_admin":false},"body":"We should not have any dependency to the physical layer in the parser. This need to be moved to the engine package. See #1965 ","created_at":"2018-10-09T18:35:55Z","updated_at":"2018-10-09T18:41:36Z","html_url":"https://github.com/confluentinc/ksql/pull/2024#discussion_r223816328","pull_request_url":"https://api.github.com/repos/confluentinc/ksql/pulls/2024","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/confluentinc/ksql/pulls/comments/223816328"},"html":{"href":"https://github.com/confluentinc/ksql/pull/2024#discussion_r223816328"},"pull_request":{"href":"https://api.github.com/repos/confluentinc/ksql/pulls/2024"}}},"pull_request":{"url":"https://api.github.com/repos/confluentinc/ksql/pulls/2024","id":221233486,"node_id":"MDExOlB1bGxSZXF1ZXN0MjIxMjMzNDg2","html_url":"https://github.com/confluentinc/ksql/pull/2024","diff_url":"https://github.com/confluentinc/ksql/pull/2024.diff","patch_url":"https://github.com/confluentinc/ksql/pull/2024.patch","issue_url":"https://api.github.com/repos/confluentinc/ksql/issues/2024","number":2024,"state":"open","locked":false,"title":"Fix session windows","user":{"login":"big-andy-coates","id":8012398,"node_id":"MDQ6VXNlcjgwMTIzOTg=","avatar_url":"https://avatars0.githubusercontent.com/u/8012398?v=4","gravatar_id":"","url":"https://api.github.com/users/big-andy-coates","html_url":"https://github.com/big-andy-coates","followers_url":"https://api.github.com/users/big-andy-coates/followers","following_url":"https://api.github.com/users/big-andy-coates/following{/other_user}","gists_url":"https://api.github.com/users/big-andy-coates/gists{/gist_id}","starred_url":"https://api.github.com/users/big-andy-coates/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/big-andy-coates/subscriptions","organizations_url":"https://api.github.com/users/big-andy-coates/orgs","repos_url":"https://api.github.com/users/big-andy-coates/repos","events_url":"https://api.github.com/users/big-andy-coates/events{/privacy}","received_events_url":"https://api.github.com/users/big-andy-coates/received_events","type":"User","site_admin":false},"body":"### Description \r\nFixes #1995\r\n\r\nQueries that used a session window clause were incorrectly having the window constituent of their key serialised as `TimeWindow` rather than `SessionWindow`.  The former does _not_ allow zero length windows, which sessions require, and does not include the window end in the serialised key. This PR looks to rectify this. \r\n\r\nHowever, this does have backwards compatibility issues that need discussing:\r\n\r\n1. The 'WINDOWED' property in the `WITH` clause is no longer supported.  This appears to be an undocumented feature that allows a windowed stream or table to be imported into KSQL with CT/CS statement, e.g. `CREATE TABLE Blah (...) WITH (... WINDOWED='true');` . This property is superseded by `WINDOW_TYPE`, which can be set to `SESSION`, `TUMBLING` or `HOPPING`.  Tests have been added to test this new functionality (the old had no tests).\r\n1. The binary format of session keys will change with this change. Previously the key contained only natural key and the window start time. With this change the key will contain both the window start and end time.  This will obviously cause issues with existing queries.  So my question is, (cc @rodesai), is it sufficient to add a `CompatibilityBreakingConfigDef` to control this behaviour, i.e. to switch between old and new serialised form?\r\n1. `ROWKEY` for session windows now includes the end of the session window. Previously, even queries with session windows would have a `ROWKEY` like `some-key Window{start=12345, end=-}`, i.e. the end time of the window was '-'. For non-session windows we generally don't know the `end` time as it is not serialised, (this is a KStream optimisation). However, as the window size is constant this is not really an issue. For session windows we _do_ know the `end` time and because the window size is not constant the `end` time is important. With this PR the end time is populated, e.g. `ROWKEY` might be `some-key Window{start=12345, end=12346}`.\r\n\r\nThis PR also allows windowed streams to be imported via `CS` statements. (They didn't work previously), e.g. you can do the following to import a windowed stream:\r\n\r\n```\r\nCREATE STREAM X WITH (kafka_topi='blah', value_format='blah', window_type='session');\r\n```\r\n\r\nThis PR supersedes #2022, (which was just about refactoring and adding tests).\r\n\r\n\r\n### Testing done \r\nThe usual.\r\n\r\n### Reviewer checklist\r\n- [ ] Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).\r\n- [ ] Ensure relevant issues are linked (description should include text like \"Fixes #<issue number>\")\r\n\r\n","created_at":"2018-10-08T20:25:54Z","updated_at":"2018-10-09T18:41:36Z","closed_at":null,"merged_at":null,"merge_commit_sha":"e7c5c95671552a7a914d52eac9d03bc0692af5a8","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/confluentinc/ksql/pulls/2024/commits","review_comments_url":"https://api.github.com/repos/confluentinc/ksql/pulls/2024/comments","review_comment_url":"https://api.github.com/repos/confluentinc/ksql/pulls/comments{/number}","comments_url":"https://api.github.com/repos/confluentinc/ksql/issues/2024/comments","statuses_url":"https://api.github.com/repos/confluentinc/ksql/statuses/477b551e9aaec85d4414ac3fc339cf1515a4084e","head":{"label":"big-andy-coates:fix_session_windows","ref":"fix_session_windows","sha":"477b551e9aaec85d4414ac3fc339cf1515a4084e","user":{"login":"big-andy-coates","id":8012398,"node_id":"MDQ6VXNlcjgwMTIzOTg=","avatar_url":"https://avatars0.githubusercontent.com/u/8012398?v=4","gravatar_id":"","url":"https://api.github.com/users/big-andy-coates","html_url":"https://github.com/big-andy-coates","followers_url":"https://api.github.com/users/big-andy-coates/followers","following_url":"https://api.github.com/users/big-andy-coates/following{/other_user}","gists_url":"https://api.github.com/users/big-andy-coates/gists{/gist_id}","starred_url":"https://api.github.com/users/big-andy-coates/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/big-andy-coates/subscriptions","organizations_url":"https://api.github.com/users/big-andy-coates/orgs","repos_url":"https://api.github.com/users/big-andy-coates/repos","events_url":"https://api.github.com/users/big-andy-coates/events{/privacy}","received_events_url":"https://api.github.com/users/big-andy-coates/received_events","type":"User","site_admin":false},"repo":{"id":122631155,"node_id":"MDEwOlJlcG9zaXRvcnkxMjI2MzExNTU=","name":"ksql","full_name":"big-andy-coates/ksql","private":false,"owner":{"login":"big-andy-coates","id":8012398,"node_id":"MDQ6VXNlcjgwMTIzOTg=","avatar_url":"https://avatars0.githubusercontent.com/u/8012398?v=4","gravatar_id":"","url":"https://api.github.com/users/big-andy-coates","html_url":"https://github.com/big-andy-coates","followers_url":"https://api.github.com/users/big-andy-coates/followers","following_url":"https://api.github.com/users/big-andy-coates/following{/other_user}","gists_url":"https://api.github.com/users/big-andy-coates/gists{/gist_id}","starred_url":"https://api.github.com/users/big-andy-coates/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/big-andy-coates/subscriptions","organizations_url":"https://api.github.com/users/big-andy-coates/orgs","repos_url":"https://api.github.com/users/big-andy-coates/repos","events_url":"https://api.github.com/users/big-andy-coates/events{/privacy}","received_events_url":"https://api.github.com/users/big-andy-coates/received_events","type":"User","site_admin":false},"html_url":"https://github.com/big-andy-coates/ksql","description":"KSQL - the Streaming SQL Engine for Apache Kafka ","fork":true,"url":"https://api.github.com/repos/big-andy-coates/ksql","forks_url":"https://api.github.com/repos/big-andy-coates/ksql/forks","keys_url":"https://api.github.com/repos/big-andy-coates/ksql/keys{/key_id}","collaborators_url":"https://api.github.com/repos/big-andy-coates/ksql/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/big-andy-coates/ksql/teams","hooks_url":"https://api.github.com/repos/big-andy-coates/ksql/hooks","issue_events_url":"https://api.github.com/repos/big-andy-coates/ksql/issues/events{/number}","events_url":"https://api.github.com/repos/big-andy-coates/ksql/events","assignees_url":"https://api.github.com/repos/big-andy-coates/ksql/assignees{/user}","branches_url":"https://api.github.com/repos/big-andy-coates/ksql/branches{/branch}","tags_url":"https://api.github.com/repos/big-andy-coates/ksql/tags","blobs_url":"https://api.github.com/repos/big-andy-coates/ksql/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/big-andy-coates/ksql/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/big-andy-coates/ksql/git/refs{/sha}","trees_url":"https://api.github.com/repos/big-andy-coates/ksql/git/trees{/sha}","statuses_url":"https://api.github.com/repos/big-andy-coates/ksql/statuses/{sha}","languages_url":"https://api.github.com/repos/big-andy-coates/ksql/languages","stargazers_url":"https://api.github.com/repos/big-andy-coates/ksql/stargazers","contributors_url":"https://api.github.com/repos/big-andy-coates/ksql/contributors","subscribers_url":"https://api.github.com/repos/big-andy-coates/ksql/subscribers","subscription_url":"https://api.github.com/repos/big-andy-coates/ksql/subscription","commits_url":"https://api.github.com/repos/big-andy-coates/ksql/commits{/sha}","git_commits_url":"https://api.github.com/repos/big-andy-coates/ksql/git/commits{/sha}","comments_url":"https://api.github.com/repos/big-andy-coates/ksql/comments{/number}","issue_comment_url":"https://api.github.com/repos/big-andy-coates/ksql/issues/comments{/number}","contents_url":"https://api.github.com/repos/big-andy-coates/ksql/contents/{+path}","compare_url":"https://api.github.com/repos/big-andy-coates/ksql/compare/{base}...{head}","merges_url":"https://api.github.com/repos/big-andy-coates/ksql/merges","archive_url":"https://api.github.com/repos/big-andy-coates/ksql/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/big-andy-coates/ksql/downloads","issues_url":"https://api.github.com/repos/big-andy-coates/ksql/issues{/number}","pulls_url":"https://api.github.com/repos/big-andy-coates/ksql/pulls{/number}","milestones_url":"https://api.github.com/repos/big-andy-coates/ksql/milestones{/number}","notifications_url":"https://api.github.com/repos/big-andy-coates/ksql/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/big-andy-coates/ksql/labels{/name}","releases_url":"https://api.github.com/repos/big-andy-coates/ksql/releases{/id}","deployments_url":"https://api.github.com/repos/big-andy-coates/ksql/deployments","created_at":"2018-02-23T14:24:40Z","updated_at":"2018-02-23T14:24:43Z","pushed_at":"2018-10-09T17:24:31Z","git_url":"git://github.com/big-andy-coates/ksql.git","ssh_url":"git@github.com:big-andy-coates/ksql.git","clone_url":"https://github.com/big-andy-coates/ksql.git","svn_url":"https://github.com/big-andy-coates/ksql","homepage":"","size":10798,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"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":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"confluentinc:master","ref":"master","sha":"3f8eff1bb2e5dce627c6861919ad4869a373b52e","user":{"login":"confluentinc","id":9439498,"node_id":"MDEyOk9yZ2FuaXphdGlvbjk0Mzk0OTg=","avatar_url":"https://avatars0.githubusercontent.com/u/9439498?v=4","gravatar_id":"","url":"https://api.github.com/users/confluentinc","html_url":"https://github.com/confluentinc","followers_url":"https://api.github.com/users/confluentinc/followers","following_url":"https://api.github.com/users/confluentinc/following{/other_user}","gists_url":"https://api.github.com/users/confluentinc/gists{/gist_id}","starred_url":"https://api.github.com/users/confluentinc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/confluentinc/subscriptions","organizations_url":"https://api.github.com/users/confluentinc/orgs","repos_url":"https://api.github.com/users/confluentinc/repos","events_url":"https://api.github.com/users/confluentinc/events{/privacy}","received_events_url":"https://api.github.com/users/confluentinc/received_events","type":"Organization","site_admin":false},"repo":{"id":69400326,"node_id":"MDEwOlJlcG9zaXRvcnk2OTQwMDMyNg==","name":"ksql","full_name":"confluentinc/ksql","private":false,"owner":{"login":"confluentinc","id":9439498,"node_id":"MDEyOk9yZ2FuaXphdGlvbjk0Mzk0OTg=","avatar_url":"https://avatars0.githubusercontent.com/u/9439498?v=4","gravatar_id":"","url":"https://api.github.com/users/confluentinc","html_url":"https://github.com/confluentinc","followers_url":"https://api.github.com/users/confluentinc/followers","following_url":"https://api.github.com/users/confluentinc/following{/other_user}","gists_url":"https://api.github.com/users/confluentinc/gists{/gist_id}","starred_url":"https://api.github.com/users/confluentinc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/confluentinc/subscriptions","organizations_url":"https://api.github.com/users/confluentinc/orgs","repos_url":"https://api.github.com/users/confluentinc/repos","events_url":"https://api.github.com/users/confluentinc/events{/privacy}","received_events_url":"https://api.github.com/users/confluentinc/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/confluentinc/ksql","description":"KSQL - the Streaming SQL Engine for Apache Kafka ","fork":false,"url":"https://api.github.com/repos/confluentinc/ksql","forks_url":"https://api.github.com/repos/confluentinc/ksql/forks","keys_url":"https://api.github.com/repos/confluentinc/ksql/keys{/key_id}","collaborators_url":"https://api.github.com/repos/confluentinc/ksql/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/confluentinc/ksql/teams","hooks_url":"https://api.github.com/repos/confluentinc/ksql/hooks","issue_events_url":"https://api.github.com/repos/confluentinc/ksql/issues/events{/number}","events_url":"https://api.github.com/repos/confluentinc/ksql/events","assignees_url":"https://api.github.com/repos/confluentinc/ksql/assignees{/user}","branches_url":"https://api.github.com/repos/confluentinc/ksql/branches{/branch}","tags_url":"https://api.github.com/repos/confluentinc/ksql/tags","blobs_url":"https://api.github.com/repos/confluentinc/ksql/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/confluentinc/ksql/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/confluentinc/ksql/git/refs{/sha}","trees_url":"https://api.github.com/repos/confluentinc/ksql/git/trees{/sha}","statuses_url":"https://api.github.com/repos/confluentinc/ksql/statuses/{sha}","languages_url":"https://api.github.com/repos/confluentinc/ksql/languages","stargazers_url":"https://api.github.com/repos/confluentinc/ksql/stargazers","contributors_url":"https://api.github.com/repos/confluentinc/ksql/contributors","subscribers_url":"https://api.github.com/repos/confluentinc/ksql/subscribers","subscription_url":"https://api.github.com/repos/confluentinc/ksql/subscription","commits_url":"https://api.github.com/repos/confluentinc/ksql/commits{/sha}","git_commits_url":"https://api.github.com/repos/confluentinc/ksql/git/commits{/sha}","comments_url":"https://api.github.com/repos/confluentinc/ksql/comments{/number}","issue_comment_url":"https://api.github.com/repos/confluentinc/ksql/issues/comments{/number}","contents_url":"https://api.github.com/repos/confluentinc/ksql/contents/{+path}","compare_url":"https://api.github.com/repos/confluentinc/ksql/compare/{base}...{head}","merges_url":"https://api.github.com/repos/confluentinc/ksql/merges","archive_url":"https://api.github.com/repos/confluentinc/ksql/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/confluentinc/ksql/downloads","issues_url":"https://api.github.com/repos/confluentinc/ksql/issues{/number}","pulls_url":"https://api.github.com/repos/confluentinc/ksql/pulls{/number}","milestones_url":"https://api.github.com/repos/confluentinc/ksql/milestones{/number}","notifications_url":"https://api.github.com/repos/confluentinc/ksql/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/confluentinc/ksql/labels{/name}","releases_url":"https://api.github.com/repos/confluentinc/ksql/releases{/id}","deployments_url":"https://api.github.com/repos/confluentinc/ksql/deployments","created_at":"2016-09-27T21:30:15Z","updated_at":"2018-10-09T18:33:22Z","pushed_at":"2018-10-09T17:24:33Z","git_url":"git://github.com/confluentinc/ksql.git","ssh_url":"git@github.com:confluentinc/ksql.git","clone_url":"https://github.com/confluentinc/ksql.git","svn_url":"https://github.com/confluentinc/ksql","homepage":"","size":11492,"stargazers_count":1943,"watchers_count":1943,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":366,"mirror_url":null,"archived":false,"open_issues_count":377,"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":366,"open_issues":377,"watchers":1943,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/confluentinc/ksql/pulls/2024"},"html":{"href":"https://github.com/confluentinc/ksql/pull/2024"},"issue":{"href":"https://api.github.com/repos/confluentinc/ksql/issues/2024"},"comments":{"href":"https://api.github.com/repos/confluentinc/ksql/issues/2024/comments"},"review_comments":{"href":"https://api.github.com/repos/confluentinc/ksql/pulls/2024/comments"},"review_comment":{"href":"https://api.github.com/repos/confluentinc/ksql/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/confluentinc/ksql/pulls/2024/commits"},"statuses":{"href":"https://api.github.com/repos/confluentinc/ksql/statuses/477b551e9aaec85d4414ac3fc339cf1515a4084e"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"hjafarpour"}}