Skip to content

[connectors] Improved tests for Avro connectors#6619

Open
mihaibudiu wants to merge 1 commit into
feldera:mainfrom
mihaibudiu:avro
Open

[connectors] Improved tests for Avro connectors#6619
mihaibudiu wants to merge 1 commit into
feldera:mainfrom
mihaibudiu:avro

Conversation

@mihaibudiu

Copy link
Copy Markdown
Contributor

The tests include fixing a bug in the output Avro connector configuration.
The focus was on covering TIMESTAMP WITH TIME ZONE.

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

@mythical-fred mythical-fred left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. The timestamp_tz_format was silently defaulting to the string format, so TIMESTAMP WITH TIME ZONE columns produced invalid Avro against timestamp-{micros,millis} schemas — good catch. Round-trip via gen_raw_parser_test covers the encoder-side fix, and the wire-format test pins down the decode direction.

@mythical-fred mythical-fred left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tightens Avro coverage for TIMESTAMP WITH TIME ZONE.

serializer.rs now sets with_timestamp_tz_format(MicrosSinceEpoch) — previously the timestamp-tz format was left at its default, which is a real gap on the output side that the new tests would have surfaced eventually. Worth noting this is a small behaviour fix, not just tests.

Coverage is solid:

  • Round-trip tests for both timestamp-micros and timestamp-millis Avro logical types, with sub-millisecond values on the micros path so precision is actually exercised.
  • test_timestamp_tz_wire_format decodes hand-built Value::TimestampMillis / Value::TimestampMicros records against hard-coded expected TimestampTz — good sanity check that the decoder isn't losing units.
  • End-to-end kafka avro workload gains a timestamp with time zone column, with decimal tightened to decimal(10, 2) for stable Avro representation.

Clean commit, signed-off, no AI trailer. LGTM.

@mihaibudiu

Copy link
Copy Markdown
Contributor Author

@ryzhyk do we need these tests in addition to the ones you wrote?

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