Skip to content

KAFKA-6605 fix NPE in Flatten when optional Struct is null - backport…#5706

Closed
mihbor wants to merge 1 commit intoapache:2.0from
mihbor:KAFKA-6605-2.0
Closed

KAFKA-6605 fix NPE in Flatten when optional Struct is null - backport…#5706
mihbor wants to merge 1 commit intoapache:2.0from
mihbor:KAFKA-6605-2.0

Conversation

@mihbor
Copy link
Copy Markdown
Contributor

@mihbor mihbor commented Sep 27, 2018

… to 2.0

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

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

Schema updatedSchema = schemaUpdateCache.get(schema);
if (updatedSchema == null) {
final SchemaBuilder builder = SchemaUtil.copySchemaBasics(value.schema(), SchemaBuilder.struct());
Struct defaultValue = (Struct) value.schema().defaultValue();
Copy link
Copy Markdown

@HungUnicorn HungUnicorn Jul 9, 2019

Choose a reason for hiding this comment

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

it looks for me that before this change, only value.schema is used instead of key.schema should also be considered, which looks not correct, or?

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.

The operatingSchema(record) (line 141 in the PR) obtains the key or value schema, depending upon which transformation is used. So the new PR is a bit more clear, and I believe it is correct.

The prior code used value for the operating value, but if the Flatten$Key transformation was being used the operatingValue(...) actually returned the key, so the value variable name is a bit of a misnomer.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for your detail explanation!

@rhauch
Copy link
Copy Markdown
Contributor

rhauch commented Jul 12, 2019

This is the effectively the same PR as #5705, and we only need to create separate PRs for older branches if backporting from newer branches isn't straightforward. Because backporting #5075 to the 2.0 branch was straightforward, I'm closing this without merging.

@rhauch rhauch closed this Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants