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

Should we use to_avro instead of RawLiteral indirection layer #562

Open
ZENOTME opened this issue Aug 18, 2024 · 7 comments
Open

Should we use to_avro instead of RawLiteral indirection layer #562

ZENOTME opened this issue Aug 18, 2024 · 7 comments

Comments

@ZENOTME
Copy link
Contributor

ZENOTME commented Aug 18, 2024

BTW, why to we need the RawLiteral indirection layer, instead of having sth like

impl Literal {
	fn to_avro(&self) -> avro::Value {...}
}

Originally posted by @xxchan in #456 (comment)

@ZENOTME
Copy link
Contributor Author

ZENOTME commented Aug 18, 2024

Using RawLiteral as an indirection layer to convert iceberg value to Avro value is not convenient, we should infer how to convert from looking at how Avro Serializer works. I'm reflecting on why we don't use fn to_avro(&self) -> avro::Value {...} directly.🤔 cc @liurenjie1024 @Xuanwo @Fokko

@liurenjie1024
Copy link
Collaborator

I think the original motivation of this indirection layer is to build a middle layer for all kinds of format, including json, binary and avro. But it seems that this is not working well as expected. I'm +1 for have different serializer for different formats, since they already have a middle layer such as avro::Value, json::Value, and it would not introduce much burden to us.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 19, 2024

Agreed. By the way, will this change introduce extra cost?

@liurenjie1024
Copy link
Collaborator

Agreed. By the way, will this change introduce extra cost?

What kind of cost do you mean?

@Xuanwo
Copy link
Member

Xuanwo commented Aug 19, 2024

What kind of cost do you mean?

I'm not sure if I understand correctly:

Literal -> Bytes vs Literal -> avro::Value -> Bytes

@xxchan
Copy link
Contributor

xxchan commented Aug 19, 2024

I think it's

Literal -> RawLiteral -> Bytes vs Literal -> avro::Value -> Bytes

@Xuanwo
Copy link
Member

Xuanwo commented Aug 19, 2024

I think it's

Literal -> RawLiteral -> Bytes vs Literal -> avro::Value -> Bytes

I see. Thanks!

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

No branches or pull requests

4 participants