-
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
Json keys #6292
Json keys #6292
Conversation
wip: confluentinc#6215 fixes: confluentinc#6214 First pass of adding QTT tests around the JSON key format, including adding tests for `DECIMAL` and `BOOLEAN` types, which is the first time they've been used as a key column type. The test around a JSON `DOUBLE` key is currently disabled, due to a limitation of QTT which reads doubles as `BigDecimal`. Code works, just need to enhance QTT to do the right thing. Commit includes a fix in `SerdeOptionsFactory` to ensure no key unwrapping is set if there is no key, plus refactored `SerdeOptionsFactory`, combining the two public methods.
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.
LGTM though I think it's worth adding a few QTTs for converting between different key formats, both positive and negative such as validating what happens if I start with a JSON boolean key and try converting the key format to KAFKA.
ksqldb-functional-tests/src/test/resources/query-validation-tests/json.json
Show resolved
Hide resolved
Conflicting files ksqldb-engine/src/main/java/io/confluent/ksql/ddl/commands/CreateSourceFactory.java ksqldb-engine/src/main/java/io/confluent/ksql/planner/LogicalPlanner.java ksqldb-engine/src/main/java/io/confluent/ksql/schema/ksql/inference/SchemaRegisterInjector.java ksqldb-engine/src/main/java/io/confluent/ksql/serde/SerdeOptionsFactory.java ksqldb-engine/src/main/java/io/confluent/ksql/structured/SchemaKGroupedStream.java ksqldb-engine/src/test/java/io/confluent/ksql/serde/SerdeOptionsFactoryTest.java ksqldb-engine/src/test/java/io/confluent/ksql/structured/SchemaKGroupedStreamTest.java ksqldb-functional-tests/src/main/java/io/confluent/ksql/test/tools/TestCaseBuilderUtil.java
See #6345 for a test on converting between formats and where the key SQL type isn't supported by the sink key schema. |
Description
part of: #6215
fixes: #6214
First pass of adding QTT tests around the JSON key format, including adding tests for
DECIMAL
andBOOLEAN
types, which is the first time they've been used as a key column type.Commit includes a fix in
SerdeOptionsFactory
to ensure no key unwrapping is set if there is no key, plus refactoredSerdeOptionsFactory
, combining the two public methods.Testing done
usual
Reviewer checklist