Skip to content

KAFKA-16288, KAFKA-16289: Fix Values convertToDecimal exception and parseString corruption#15399

Merged
gharris1727 merged 5 commits into
apache:trunkfrom
gharris1727:kafka-16288-kafka-16289-values-fixes
Mar 4, 2024
Merged

KAFKA-16288, KAFKA-16289: Fix Values convertToDecimal exception and parseString corruption#15399
gharris1727 merged 5 commits into
apache:trunkfrom
gharris1727:kafka-16288-kafka-16289-values-fixes

Conversation

@gharris1727

Copy link
Copy Markdown
Contributor

See the descriptions of the tickets for full details:

These both represent breaking changes in behavior, but only when using incompatible-type arrays and maps, such as the ones included in the tests. Since the behavior of these is so opaque and silent corruption is possible with the bugs, we should change the behavior unconditionally.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

…tToDecimal

Signed-off-by: Greg Harris <greg.harris@aiven.io>
… element order

Signed-off-by: Greg Harris <greg.harris@aiven.io>
Comment on lines +810 to +811
assertEquals(Type.ARRAY, schema.type());
assertNull(schema.valueSchema());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it too difficult to also add an assertion about the parsed values in the array? Not a blocker, but seems nice to have if possible, especially since we don't cover anything except various representations of 1 in the other test above.

@C0urante C0urante left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks Greg!

@gharris1727

Copy link
Copy Markdown
Contributor Author

Test failures appear unrelated, and the tests pass for me locally.

@gharris1727 gharris1727 merged commit 99e511c into apache:trunk Mar 4, 2024
testn pushed a commit to testn/kafka that referenced this pull request Mar 15, 2024
…arseString corruption (apache#15399)

* KAFKA-16288: Prevent ClassCastExceptions for strings in Values.convertToDecimal
* KAFKA-16289: Values inferred schemas for map and arrays should ignore element order

Signed-off-by: Greg Harris <greg.harris@aiven.io>
Reviewers: Chris Egerton <chrise@aiven.io>
clolov pushed a commit to clolov/kafka that referenced this pull request Apr 5, 2024
…arseString corruption (apache#15399)

* KAFKA-16288: Prevent ClassCastExceptions for strings in Values.convertToDecimal
* KAFKA-16289: Values inferred schemas for map and arrays should ignore element order

Signed-off-by: Greg Harris <greg.harris@aiven.io>
Reviewers: Chris Egerton <chrise@aiven.io>
Phuc-Hong-Tran pushed a commit to Phuc-Hong-Tran/kafka that referenced this pull request Jun 6, 2024
…arseString corruption (apache#15399)

* KAFKA-16288: Prevent ClassCastExceptions for strings in Values.convertToDecimal
* KAFKA-16289: Values inferred schemas for map and arrays should ignore element order

Signed-off-by: Greg Harris <greg.harris@aiven.io>
Reviewers: Chris Egerton <chrise@aiven.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants