You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "twthorn (via GitHub)" <gi...@apache.org> on 2026/06/15 23:18:46 UTC
[PR] Kafka Connect: Fix avro schema conversion for UUID [iceberg]
twthorn opened a new pull request, #16828:
URL: https://github.com/apache/iceberg/pull/16828
Closes https://github.com/apache/iceberg/issues/12862
Fix avro schema conversion for UUID (not simple string when schema is UUID logical type)
Reproduce error with test first, then added fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Fix avro schema conversion for UUID [iceberg]
Posted by "twthorn (via GitHub)" <gi...@apache.org>.
twthorn commented on PR #16828:
URL: https://github.com/apache/iceberg/pull/16828#issuecomment-4775454785
@laskoviymishka I have one more PR here, a short one, if you can take a look. Test failures are all unrelated (for Spark, this is for kafka connect), I don't have permissions to rerun them. Thank you again for the help
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Fix avro schema conversion for UUID [iceberg]
Posted by "twthorn (via GitHub)" <gi...@apache.org>.
twthorn commented on PR #16828:
URL: https://github.com/apache/iceberg/pull/16828#issuecomment-4784699027
@laskoviymishka Thanks for the quick review. Agreed on just supporting string, from avro [spec](https://avro.apache.org/docs/1.11.1/specification/#uuid)
> A uuid logical type annotates an Avro string. The string has to conform with [RFC-4122](https://www.ietf.org/rfc/rfc4122.txt)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Fix avro schema conversion for UUID [iceberg]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on code in PR #16828:
URL: https://github.com/apache/iceberg/pull/16828#discussion_r3461140592
##########
kafka-connect/kafka-connect/src/test/java/org/apache/iceberg/connect/data/TestSchemaUtils.java:
##########
@@ -331,4 +332,20 @@ public void testInferIcebergTypeEmpty() {
assertThat(SchemaUtils.inferIcebergType(ImmutableMap.of("nested", ImmutableMap.of()), config))
.isNull();
}
+
+ @Test
+ public void testToIcebergTypeUUIDLogicalTypeOnString() {
+ IcebergSinkConfig config = mock(IcebergSinkConfig.class);
+
+ Schema uuidSchema = SchemaBuilder.string().name("uuid").build();
+ assertThat(SchemaUtils.toIcebergType(uuidSchema, config)).isInstanceOf(UUIDType.class);
+ }
+
+ @Test
+ public void testToIcebergTypeUUIDLogicalTypeOnBytes() {
+ IcebergSinkConfig config = mock(IcebergSinkConfig.class);
Review Comment:
This test passes, but it's why the BYTES bug above stays invisible — it only checks the type-inference result, never the conversion. If we keep the BYTES→UUIDType mapping, I'd want a companion test that builds a Connect Struct with a BYTES "uuid" field, runs `RecordConverter.convert`, and asserts the resulting record holds the right UUID. That's the test that fails today.
Same gap exists for the STRING path — `TestRecordConverter` only uses `Schema.STRING_SCHEMA` (no logical name), so the new STRING+uuid path is never round-tripped either. One end-to-end test per kept branch would close this.
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/data/SchemaUtils.java:
##########
@@ -235,6 +236,8 @@ Type toIcebergType(Schema valueSchema) {
if (Decimal.LOGICAL_NAME.equals(valueSchema.name())) {
int scale = Integer.parseInt(valueSchema.parameters().get(Decimal.SCALE_FIELD));
return DecimalType.of(38, scale);
+ } else if ("uuid".equals(valueSchema.name())) {
Review Comment:
I think this BYTES branch is broken at write time. We create a UUIDType column here, but `RecordConverter.convertUUID` (RecordConverter.java:413-431) only handles `String` and `java.util.UUID` — a Connect BYTES field comes through as a `ByteBuffer`, so every record throws `Cannot convert to UUID`. That's worse than the pre-fix behavior, where the data at least landed as binary.
Either we extend `convertUUID` to accept a 16-byte big-endian `ByteBuffer`/`byte[]` (`org.apache.iceberg.util.UUIDUtil.convert` handles that), or we drop this branch and leave BYTES+uuid as BinaryType with a comment. The STRING branch is the one that maps to the real-world source, so I'd lean toward dropping BYTES unless we add the converter support and a round-trip test. wdyt?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Fix avro schema conversion for UUID [iceberg]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka merged PR #16828:
URL: https://github.com/apache/iceberg/pull/16828
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Fix avro schema conversion for UUID [iceberg]
Posted by "twthorn (via GitHub)" <gi...@apache.org>.
twthorn commented on PR #16828:
URL: https://github.com/apache/iceberg/pull/16828#issuecomment-4713377557
@Fokko would appreciate your eyes on this if you can take a look. Looks like you were active on the issue. The previous PR attempted a fix in the wrong location (no bug existed in core, I verified this locally with a test, happy to commit that if we want that test coverage in core). The bug is just in the kafka-connect schema utils so fixed/reproduced there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org