-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: enforce WITH KEY column type matches ROWKEY type #4147
Merged
big-andy-coates
merged 11 commits into
confluentinc:master
from
big-andy-coates:prim_keys_with_schema
Dec 19, 2019
Merged
chore: enforce WITH KEY column type matches ROWKEY type #4147
big-andy-coates
merged 11 commits into
confluentinc:master
from
big-andy-coates:prim_keys_with_schema
Dec 19, 2019
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds support for using primitive types in joins. BREAKING CHANGE: Some existing joins may now fail and the type of `ROWKEY` in the result schema of joins may have changed. When `ROWKEY` was always a `STRING` it was possible to join an `INTEGER` column with a `BIGINT` column. This is no longer the case. A `JOIN` requires the join columns to be of the same type. (See confluentinc#4130 which tracks adding support for being able to `CAST` join criteria). Where joining on two `INT` columns would previously have resulted in a schema containing `ROWKEY STRING KEY`, it would not result in `ROWKEY INT KEY`.
BREAKING CHANGE: Any `KEY` column identified in the `WITH` clause must be of the same Sql type as `ROWKEY`. Users can provide the name of a value column that matches the key column, e.g. ```sql CREATE STREAM S (ID INT, NAME STRING) WITH (KEY='ID', ...); ``` Before primitive keys was introduced all keys were treated as `STRING`. With primitive keys `ROWKEY` can be types other than `STRING`, e.g. `BIGINT`. It therefore follows that any `KEY` column identified in the `WITH` clause must have the same SQL type as the _actual_ key, i.e. `ROWKEY`. With this change the above example statement will fail with the error: ``` The KEY field (ID) identified in the WITH clause is of a different type to the actual key column. Either change the type of the KEY field to match ROWKEY, or explicitly set ROWKEY to the type of the KEY field by adding 'ROWKEY INTEGER KEY' in the schema. KEY field type: INTEGER ROWKEY type: STRING ``` As the error message says, the error can be resolved by changing the statement to: ```sql CREATE STREAM S (ROWKEY INT KEY, ID INT, NAME STRING) WITH (KEY='ID', ...); ```
agavra
approved these changes
Dec 16, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM! Thanks @big-andy-coates
Conflicting files ksql-functional-tests/src/test/resources/query-validation-tests/joins.json
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes: #4097
Fixes: #4146
Note: stacked on top of #4132, review that first!
BREAKING CHANGE: Any
KEY
column identified in theWITH
clause must be of the same Sql type asROWKEY
.Users can provide the name of a value column that matches the key column, e.g.
Before primitive keys was introduced all keys were treated as
STRING
. With primitive keysROWKEY
can be types other thanSTRING
, e.g.BIGINT
.It therefore follows that any
KEY
column identified in theWITH
clause must have the same SQL type as the actual key, i.e.ROWKEY
.With this change the above example statement will fail with the error:
As the error message says, the error can be resolved by changing the statement to:
Testing done
Tests added and updated.
Reviewer checklist