Follow-up on arrow-avro Documentation #8402

jecsand838 · 2025-09-22T08:44:11Z

Which issue does this PR close?

Related to: Add Avro Support #4886 (“Add Avro Support”)
Follows-up on Add arrow-avro examples and Reader documentation #8316

Rationale for this change

@alamb had some recommendations for improving the arrow-avro documentation in #8316. This is a follow-up to address those suggestions.

What changes are included in this PR?

lib.rs documentation
reader/mod.rs improved documentation and inlined examples
writer/mod.rs improved documentation and inlined examples

NOTE: Some doc tests are temporarily ignored until #8371 is merged in.

Are these changes tested?

Yes, doc tests have been included which all run (with the exception of 3 ignored ones that will work soon)

Are there any user-facing changes?

N/A

…-avro`.

alamb

Thank you @jecsand838 -- this is looking great. I had a few suggestions, but otherwise it looks like a great improvement to me

alamb · 2025-09-22T19:43:21Z

arrow-avro/src/lib.rs

+//! 3) Decode it back to Arrow using a `Decoder` configured with a `SchemaStore` that
+//!    maps the schema ID to the Avro schema used by the writer.
+//!
+//! ```ignore


can we enable this one now that we have merged

Avro writer prefix support #8371

arrow-avro/src/lib.rs

alamb · 2025-09-22T19:44:46Z

arrow-avro/src/lib.rs

+//! This example registers the writer schema (computing a Rabin fingerprint), writes a
+//! single‑row Avro body, constructs the SOE frame, and decodes it back to Arrow.
+//!
+//! ```ignore


likewise here, this should now work, right and we can use the Avro writer?

Absolutely, and I just pushed up those changes.

alamb · 2025-09-22T19:45:59Z

arrow-avro/src/lib.rs

+//!   while (n & !0x7F) != 0 { out.push(((n as u8) & 0x7F) | 0x80); n >>= 7; }
+//!   out.push(n as u8);
+//! }
+//! fn enc_len(l: usize, out: &mut Vec<u8>) { enc_long(l as i64, out); }


This example doesn't' actually use an AvroStreamWriter but the text says it does.

Can we update it to use the writer?

Good callout! I cleaned this up.

alamb · 2025-09-22T19:48:42Z

arrow-avro/src/reader/mod.rs

+//!   {"name":"id","type":"long"}]}"#.to_string());
+//! let fp = store.register(avro_schema)?;
+//!
+//! // Minimal Avro long encoder (zig-zag + varint).


can we use the Avro writer here too? If the idea is to show how to use the decoder, maybe we could just hide the code that creates the input body (aka prefix all the lines related to creating the file with # )

The same thing applies to the examples below too

Absolutely. I made sure to hide the input body code as well.

alamb · 2025-09-22T19:50:20Z

arrow-avro/src/writer/mod.rs

+//!   - **Confluent wire format**: magic `0x00` + **big‑endian** 4‑byte schema ID and Avro body.
+//!     <https://docs.confluent.io/platform/current/schema-registry/fundamentals/serdes-develop/index.html#wire-format>
+//!
+//! ## Quickstart: Write an OCF in memory (runnable)


I personally suggest moving this example to the AvroWriter struct itself as I think it will be easier to find

I checked the docs before pushing and agree with you. 100% good callout!

alamb · 2025-09-22T19:50:40Z

arrow-avro/src/writer/mod.rs

+//! use std::sync::Arc;
+//! use arrow_array::{ArrayRef, Int64Array, RecordBatch};
+//! use arrow_schema::{DataType, Field, Schema};
+//! use arrow_avro::writer::AvroStreamWriter;


likewise, i suggest moving this to the docs on AvroStreamWriter as I think it will be easier to find

alamb · 2025-09-22T19:50:44Z

arrow-avro/src/writer/mod.rs

+//! If you plan to interoperate with a schema registry, add the appropriate prefix yourself
+//! (see links above).
+//!
+//! ```ignore


this can run too?

arrow-avro/src/writer/mod.rs

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

jecsand838 · 2025-09-22T21:52:49Z

Thank you @jecsand838 -- this is looking great. I had a few suggestions, but otherwise it looks like a great improvement to me

@alamb Really appreciate the solid review. I went ahead and pushed up changes that address your comments. Let me know what you think when you get a chance!

alamb · 2025-09-23T18:24:15Z

CI failure unrelated to this PR

Fix clippy #8426

alamb

Looks great to me -- thanks @jecsand838

@jecsand838

# Which issue does this PR close? Follow on to - #8402 # Rationale for this change Let's tell people about arrow-avro # What changes are included in this PR? Touchup the main crate docs to mention arrow-avro # Are these changes tested? CI and I tested the docs manually # Are there any user-facing changes? Docs only cc @jecsand838

github-actions bot added arrow Changes to the arrow crate arrow-avro arrow-avro crate labels Sep 22, 2025

jecsand838 force-pushed the avro-documentation-follow-up branch 2 times, most recently from 7aa5764 to e5cd3d8 Compare September 22, 2025 08:50

Remove outdated Avro examples and consolidate documentation in `arrow…

93b6e0a

…-avro`.

jecsand838 force-pushed the avro-documentation-follow-up branch from e5cd3d8 to 93b6e0a Compare September 22, 2025 08:52

jecsand838 added 2 commits September 22, 2025 10:45

Merge branch 'main' into avro-documentation-follow-up

6b4fc4f

Merge branch 'main' into avro-documentation-follow-up

46d535f

alamb reviewed Sep 22, 2025

View reviewed changes

jecsand838 and others added 2 commits September 22, 2025 14:52

Update arrow-avro/src/lib.rs

cb00143

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

Update arrow-avro/src/writer/mod.rs

c9f4001

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

jecsand838 force-pushed the avro-documentation-follow-up branch from 034b6e7 to 2cc0264 Compare September 22, 2025 20:42

Update arrow-avro/src/writer/mod.rs

9fe5fe4

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

jecsand838 force-pushed the avro-documentation-follow-up branch from 2cc0264 to 9fe5fe4 Compare September 22, 2025 20:51

Address PR Comments

76827d7

jecsand838 force-pushed the avro-documentation-follow-up branch from f8a11b4 to 76827d7 Compare September 22, 2025 21:49

jecsand838 requested a review from alamb September 22, 2025 21:52

jecsand838 added 2 commits September 23, 2025 12:55

Merge branch 'main' into avro-documentation-follow-up

1aebfc5

Merge branch 'main' into avro-documentation-follow-up

3ae7a91

alamb approved these changes Sep 23, 2025

View reviewed changes

alamb merged commit 3027dbc into apache:main Sep 23, 2025
22 of 23 checks passed

jecsand838 deleted the avro-documentation-follow-up branch September 23, 2025 18:38

alamb mentioned this pull request Nov 3, 2025

docs: Add links for arrow-avro #8770

Merged

Follow-up on arrow-avro Documentation #8402

Follow-up on arrow-avro Documentation #8402

Uh oh!

Conversation

jecsand838 commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jecsand838 commented Sep 22, 2025

Uh oh!

alamb commented Sep 23, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jecsand838 commented Sep 22, 2025 •

edited

Loading