Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix: Consistency between inserted rowkey and key values when inserting into a table with a key #3381

Merged
merged 6 commits into from Sep 18, 2019
Merged

Fix: Consistency between inserted rowkey and key values when inserting into a table with a key #3381

merged 6 commits into from Sep 18, 2019

Conversation

purplefox
Copy link
Contributor

Description

Fixes #3347

  • When inserting into a TABLE with a key, and specifying the key field but not the rowkey, we ensure that the rowkey is set to the string form of the key
  • When inserting into a TABLE with a key and specifying both the rowkey and the key, we check that the rowkey matches the string form of the key

Testing done

Added several new RQTT tests to test various combinations of inserting with rowkey and key for different types.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

Tim Fox added 3 commits September 18, 2019 14:29
fix:confluentinc#3347 If the table has an key and a value for it has …
d49f282 
…been provided in an insert then make sure rowkey gets the string form of it
fix: When ROWKEY and ID are both specified in insert into, then check…
f141cc1 
… rowkey is string form of id
fix: Add a bunch of RQTT tests to test combinations of inserts with i…
fb05124 
…d and rowkey when stream has key
@purplefox purplefox requested a review from a team as a code owner September 18, 2019 14:42
big-andy-coates
big-andy-coates approved these changes Sep 18, 2019
View reviewed changes
Copy link
Contributor

@big-andy-coates big-andy-coates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @purplefox

LGTM, 'cept a few nits

ksql-engine/src/main/java/io/confluent/ksql/engine/InsertValuesExecutor.java Outdated Show resolved Hide resolved
ksql-engine/src/main/java/io/confluent/ksql/engine/InsertValuesExecutor.java Outdated Show resolved Hide resolved
ksql-functional-tests/src/test/resources/rest-query-validation-tests/insert-values.json Outdated Show resolved Hide resolved
ksql-functional-tests/src/test/resources/rest-query-validation-tests/insert-values.json Outdated Show resolved Hide resolved
ksql-functional-tests/src/test/resources/rest-query-validation-tests/insert-values.json Outdated Show resolved Hide resolved
ksql-functional-tests/src/test/resources/rest-query-validation-tests/insert-values.json Outdated Show resolved Hide resolved
@purplefox purplefox merged commit a909737 into confluentinc:master Sep 18, 2019
@purplefox purplefox deleted the fix-3347 branch September 18, 2019 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@big-andy-coates big-andy-coates big-andy-coates approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

INSERT VALUES fails if key field is not STRING
2 participants
@purplefox @big-andy-coates