refactor: do not allow duplicates in schemas by default #4697
Conversation
KsqlDB does not support data sources with duplicate column names in the key and value. (Mainly due to the fact that it copies the key columns into the value while processing with Kafka Streams). It therefore makes sense, to me at least, that our `LogicalSchema` rejects duplicates by default, but allows them if explicitly told to do so. Doesn't change anything functionally, but does mean code is more explicit about when it does and does not allow duplicates.
How does it fail currently? |
Because key columns must be named However, I'm dropping the requirement that key columns must be called |
|
ah yes... that makes sense. In that case I think this PR is good - i'll take a look at the code itself in a bit |
| * | ||
| * @return self. | ||
| */ | ||
| public Builder allowDuplicates() { |
There was a problem hiding this comment.
as discussed offline, I think having this makes things a little more confusing. If we wanted to make it strongly typed, I think we should have a different class that allows duplicates but I think that's overkill
There was a problem hiding this comment.
Yep, cool, my thinking was going down the same route.
Description
KsqlDB does not support data sources with duplicate column names in the key and value. (Mainly due to the fact that it copies the key columns into the value while processing with Kafka Streams).
It therefore makes sense, to me at least, that our
LogicalSchemarejects duplicates by default, but allows them if explicitly told to do so. Doesn't change anything functionally, but does mean code is more explicit about when it does and does not allow duplicates.Thoughts? Is this an improvement or just noise?
Testing done
Usual.
Reviewer checklist