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.

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

feat: add BYTES type to ksqlDB #7778

Merged
merged 4 commits into from
Jul 14, 2021
Merged

Conversation

jzaralim
Copy link
Contributor

@jzaralim jzaralim commented Jul 9, 2021

Description

Adds SqlTypes.BYTES and BytesLiteral to ksqlDB. The main KLIP, #7764, is not merged yet but this PR is almost 100% wiring with no logic, so I'm filing this now to prep for future BYTES work.

Testing done

Updated unit tests

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 #")

@jzaralim jzaralim requested a review from spena July 9, 2021 23:32
@jzaralim jzaralim requested a review from a team as a code owner July 9, 2021 23:32
@Immutable
public class BytesLiteral extends Literal {

private final ImmutableList<Byte> value;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing this because ByteBuffer and byte[] are both mutable.

Comment on lines 39 to 41
final byte[] bytes = new byte[requireNonNull(value, "value").capacity()];
value.get(bytes);
this.value = ImmutableList.copyOf(ArrayUtils.toObject(bytes));
Copy link
Member

Choose a reason for hiding this comment

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

What if you keep the final byte[] bytes at the class variable instead of the ImmutableList<Byte>? The value.get(bytes) will copy everything from ByteBuffer -> bytes, so you have an exact immutable copy.

Later, when you call getValue(), you can return ByteBuffer.wrap(bytes).asReadOnlyBuffer() to avoid the returned byte buffer can be modified.

And, when you call getByteArray(), then clone the array

return bytes == null ? null : bytes.clone();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that works!

Copy link
Member

@spena spena left a comment

Choose a reason for hiding this comment

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

LGTM

@jzaralim jzaralim merged commit 06657ba into confluentinc:master Jul 14, 2021
@jzaralim jzaralim deleted the bytes_type branch July 14, 2021 18:26
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.

2 participants