Added List and Struct Encoding to arrow-avro Writer #8274

jecsand838 · 2025-09-03T04:17:15Z

Which issue does this PR close?

Part of Add Avro Support #4886

Rationale for this change

This refactor streamlines the arrow-avro writer by introducing a single, schema‑driven RecordEncoder that plans writes up front and encodes rows using consistent, explicit rules for nullability and type dispatch. It reduces duplication in nested/struct/list handling, makes the order of Avro union branches (null‑first vs null‑second) an explicit choice, and aligns header schema generation with value encoding.

This should improve correctness (especially for nested optionals), make behavior easier to reason about, and pave the way for future optimizations.

What changes are included in this PR?

High‑level:

Introduces a unified, schema‑driven RecordEncoder with a builder that walks the Avro record in Avro order and maps each field to its Arrow column, producing a reusable write plan. The encoder covers scalars and nested types (struct, (large) lists, maps, strings/binaries).
Applies a single model of nullability throughout encoding, including nested sites (list items, fixed‑size list items, map values), and uses explicit union‑branch indices according to the chosen order.

API and implementation details:

Writer / encoder refactor
- Replaces the previous per‑column/child encoding paths with a FieldPlan tree (variants for Scalar, Struct { … }, and List { … }) and per‑site nullability carried from the Avro schema.
- Adds encoder variants for LargeBinary, Utf8, Utf8Large, List, LargeList, and Struct.
- Encodes union branch indices with write_optional_index (writes 0x00/0x02 according to Null‑First/Null‑Second), replacing the old branch write.
Schema generation & metadata
- Moves the Nullability enum to schema.rs and threads it through schema generation and writer logic.
- Adds AvroSchema::from_arrow_with_options(schema, Option<Nullability>) to either reuse embedded Avro JSON or build new Avro JSON that honors the requested null‑union order at all nullable sites.
- Adds extend_with_passthrough_metadata so Arrow schema metadata is copied into Avro JSON while skipping Avro‑reserved and internal Arrow keys.
- Introduces helpers like wrap_nullable and arrow_field_to_avro_with_order to apply ordering consistently for arrays, fixed‑size lists, maps, structs, and unions.
Format and glue
- Simplifies writer/format.rs by removing the EncoderOptions plumbing from the OCF format; write_long remains exported for header writing.

Are these changes tested?

Yes.

Adds focused unit tests in writer/encoder.rs that verify scalar and string/binary encodings (e.g., Binary/LargeBinary, Utf8/LargeUtf8) and validate length/branch encoding primitives used by the writer.
Round trip integration tests that validate List and Struct decoding in writer/mod.rs.
Adjusts existing schema tests (e.g., decimal metadata expectations) to align with the new schema/metadata handling.

Are there any user-facing changes?

N/A because arrow-avro is not public yet.

jecsand838 · 2025-09-03T04:22:39Z

arrow-avro/src/codec.rs

-/// Avro types are not nullable, with nullability instead encoded as a union
-/// where one of the variants is the null type.
-///
-/// To accommodate this we special case two-variant unions where one of the
-/// variants is the null type, and use this to derive arrow's notion of nullability
-#[derive(Debug, Copy, Clone, PartialEq)]
-pub enum Nullability {
-    /// The nulls are encoded as the first union variant
-    NullFirst,
-    /// The nulls are encoded as the second union variant
-    NullSecond,
-}


I moved this to src/schema.rs to keep the imports clean and non-circular.

jecsand838 · 2025-09-03T04:31:11Z

@scovich @alamb Would either of you be able to review this one? This should be the last large PR for arrow-avro as it lays the full foundation for the Writer. I tried to slim it down as much as possible.

nathaniel-d-ef · 2025-09-03T13:30:52Z

arrow-avro/src/schema.rs

+                name_gen,
+                order,
+            )?;
+            let items_with_meta = merge_extras(items_inner, ie);


Perhaps worth it to put this nullable logic in datatype_to_avro_with_order as it's duplicated here and in Map?

Solid catch, @scovich caught this one too.

arrow-avro/src/writer/encoder.rs

nathaniel-d-ef · 2025-09-03T14:03:17Z

arrow-avro/src/writer/encoder.rs

+    #[inline]
+    fn precomputed_union_value_branch(&self, order: Option<Nullability>) -> Option<u8> {
+        match (order, self.has_nulls()) {
+            (Some(Nullability::NullFirst), false) => Some(0x02), // value branch index 1


This is explained elsewhere, I think we can eliminate the comments. Maybe alias the values as constants globally.

nathaniel-d-ef · 2025-09-03T14:12:48Z

arrow-avro/src/writer/format.rs

            None => "null",
        };
-        write_long(writer, 2)?;
+        write_long(writer, 2)?; // two entries


This note seems unnecessary

…implify field handling. Includes encoder type dispatch improvements, nullability support, and schema-driven write plans.

scovich

Flushing a partial review. I got all the files except encoder.rs, which is partially reviewed.

scovich · 2025-09-03T11:30:58Z

arrow-avro/src/schema.rs

+        let record_name = schema
+            .metadata
+            .get(AVRO_NAME_METADATA_KEY)
+            .map_or("topLevelRecord", |s| s.as_str());


aside: Is this a well-known default name? Or just an arbitrary naming choice by this package? And does it actually matter in practice? (I guess if it mattered, the schema metadata would say so)?

aside: Is this a well-known default name? Or just an arbitrary naming choice by this package?

While not an Avro‑spec default, topLevelRecord is used as a de‑facto default because several popular tools (notably Spark/Databricks) default to the same name when they synthesize an Avro schema from a struct/row.

And does it actually matter in practice? (I guess if it mattered, the schema metadata would say so)?

Avro requires that a record have a name to be valid. Also because the record name participates in canonical form parsing, changing a record's name will change it's fingerprint.

So do we need to make this tunable somehow? Or the de facto default is good enough in practice?

I think it's good enough in practice, but we can always follow-up on this if needed.

arrow-avro/src/schema.rs

scovich · 2025-09-03T11:39:36Z

arrow-avro/src/schema.rs

+fn arrow_field_to_avro(
+    field: &ArrowField,
+    name_gen: &mut NameGenerator,
+) -> Result<Value, ArrowError> {
+    arrow_field_to_avro_with_order(field, name_gen, Nullability::NullFirst)
+}


Why not define this down below, similar to what you did here with data_type_to_avro[_with_order]?

arrow-avro/src/schema.rs

scovich · 2025-09-03T12:13:49Z

arrow-avro/src/schema.rs

+            let (items_inner, ie) = datatype_to_avro_with_order(
+                child.data_type(),
+                child.name(),
+                child.metadata(),
+                name_gen,
+                order,
+            )?;
+            let items_with_meta = merge_extras(items_inner, ie);
+            let items_schema = if child.is_nullable() {
+                wrap_nullable(items_with_meta, order)
+            } else {
+                items_with_meta
+            };


This exact pattern shows up four times (List/LargeList, FixedSizedList, Map, and struct field). Can we pull out a helper that all four call sites can use? It might look something like this:

fn process_datatype_with_nullability( dt: &DataType, field_name: &str, metadata: &HashMap<String, String>, name_gen: &mut NameGenerator, null_order: Nullability, is_nullable: bool, ) -> Result<Value, ArrowError> { let (schema, extras) = datatype_to_avro_with_order(dt, field_name, metadata, name_gen, null_order)?; let schema = merge_extras(schema, extras); if is_nullable { wrap_nullable(schema, null_union_order) } else { Ok(schema) } }

(terrible name for the helper, but you get the idea)

There are two additional sites (Dictionary and RunEndEncoded) that could also use the helper -- they'd just need to pass is_nullable: false.

If you're looking for ways to split up the PR, a useful "prerefactor" might:

plumb through the new (largely unused) order arg in datatype_to_avro_with_order and friends

add+use this new helper at the 4-6 call sites that benefit from it

Solid catch, I'll add that in!

scovich · 2025-09-03T13:35:22Z

arrow-avro/src/writer/encoder.rs

-    impala_mode: bool, // Will be fully implemented in a follow-up PR
-}
+/// Plan reference passed to the unified encoder constructor (required).
+type PlanRef<'p> = &'p FieldPlan;


I'm not sure this is a helpful type alias? It only has two use sites, both as function arg types with elided lifetimes. With the alias they look like this, because lifetime elision can't fully simplify the signature:

plan: PlanRef<'_>,

vs. without the alias and complete lifetime elision:

plan: &Plan,

Good catch!

arrow-avro/src/writer/encoder.rs

scovich · 2025-09-03T13:58:58Z

arrow-avro/src/writer/encoder.rs

+        })
+    }
+
+    #[inline]


I see a lot of these #[inline] markers, but the compiler already inlines code pretty aggressively.
Do we have some evidence that these markers actually improve performance?

That's a good callout. Before I push up my changes, I'll benchmark with and without the #[inline] markers to determine whether they add value or not.

I wouldn't be shocked if one or two inline markers actually matter... but no idea which two. Can we just remove all the markers and track a follow-up issue to make profile-guided decisions on which ones to bring back?

Removing the inlines caused about a ~4% decrease in throughput performance.

I bet profile-guided optimizations would give back far more than 4%, since it covers far more than just inlining decisions. If we care about optimizing that much, we should probably go that route? But that requires benchmarking and profiling infra that arrow-rs probably lacks. An hybrid (and probably almost optimal) approach would be to manually gather .profdata, and then see what llvm-profdata show has to say about these functions.

Humans are provably terrible at guessing where to sprinkle hints like inlining or branch prediction, and so I would expect that some of the inline markers are badly-placed and actually hurting performance -- especially for real-world (non-microbenchmark) scenarios where CPU instruction cache capacity matters -- which eats into the performance gains of the well-placed markers. Automatic or manual profile-guided optimization at least gives some concrete data to replace those guesses.

Overall, my recommended approach would be to remove all the inline markers for now (focus on getting correct code in), and if we want to chase the 4% that could be a follow-up item to do it right. Preferably after the code has stabilized, since it's still evolving rapidly right now.

100% fair point. I went ahead and removed them.

arrow-avro/src/writer/encoder.rs

scovich · 2025-09-03T14:15:20Z

arrow-avro/src/writer/encoder.rs

+        out
+    }
+
+    fn encode_all(array: &dyn Array, plan: &FieldPlan, site: Option<Nullability>) -> Vec<u8> {


site is a really unintuitive name for this... earlier code used site_nullability which seems better?
But why not just nullability for both?

I'll rename to nullability, that's a good recommendation.

Co-authored-by: Ryan Johnson <[email protected]>

jecsand838 · 2025-09-04T22:26:34Z

@scovich @nathaniel-d-ef I really appreciate the solid reviews.

The changes I just pushed should address most of your feedback. There are two items I wanted to follow-up on in future PRs:

Further arrow-avro/src/schema.rs file refactoring (if possible). As I updated the file I saw other potential areas to improve. For example, I think if we introduced some sort of struct to hold context for datatype_to_avro, we could simplify and improve the logic.
Renaming Nullability. The name change will impact other areas of the code not related to the Writer. I'd also want to consider the impact Union type support will have as well when we get there. But I do think @scovich has a valid point about the current naming convention.

Let me know what you all think and what else needs to be done to get this PR into solid shape whenever you get a chance.

scovich

Bunch of nits to simplify the code, but otherwise ready to merge.

There are two items I wanted to follow-up on in future PRs:

1. Further `arrow-avro/src/schema.rs` file refactoring (if possible). As I updated the file I saw other potential areas to improve. For example, I think if we introduced some sort of struct to hold context for `datatype_to_avro`, we could simplify and improve the logic.

Definitely future work.

2. Renaming `Nullability`. The name change will impact other areas of the code not related to the `Writer`. I'd also want to consider the impact `Union` type support will have as well when we get there. But I do think @scovich has a valid point about the current naming convention.

Yes. Mixing refactors with new functionality is always messy.

I would recommend two other follow-ups:
3. Profile-guided placement of #[inline] markers
4. Harmonize arg ordering for the various encoding methods that take out; some pass it as the first arg, others as the second arg.

arrow-avro/src/schema.rs

arrow-avro/src/writer/encoder.rs

scovich · 2025-09-05T14:05:53Z

arrow-avro/src/writer/encoder.rs

+                let null_buffer = array.nulls().cloned().ok_or_else(|| {
+                    ArrowError::InvalidArgumentError(format!(
+                        "Array for Avro site '{}' reports nulls but has no null buffer",
+                        field.name()
+                    ))
+                })?;


aside: I'm not sure ok_or_else plus ? is actually helpful... unless the thirteen extra chars from return Err(...); makes the inner part spill to multiple lines (which it doesn't, in this case)

Suggested change

let null_buffer = array.nulls().cloned().ok_or_else(|| {

ArrowError::InvalidArgumentError(format!(

"Array for Avro site '{}' reports nulls but has no null buffer",

field.name()

))

})?;

let Some(nulls) = array.nulls().cloned() else {

return Err(ArrowError::InvalidArgumentError(format!(

"Array for Avro site '{}' reports nulls but has no null buffer",

field.name()

)));

};

arrow-avro/src/writer/encoder.rs

scovich · 2025-09-05T14:47:44Z

arrow-avro/src/writer/encoder.rs

+    // Emit a single positive block for performance, then the end marker.
+    write_long(out, len as i64)?;
+    for j in start..end {
+        write_item(j, out)?;


aside: It's a bit disorienting that some methods (like this one and encoder encode) take out as the second param, while others (like write_long below) take out as the first param? Can we harmonize them? I would suggest out is always the first param, but consistency is the main concern.

That's very fair, I'll clean that up.

Co-authored-by: Ryan Johnson <[email protected]>

jecsand838 · 2025-09-05T18:53:36Z

Bunch of nits to simplify the code, but otherwise ready to merge.
There are two items I wanted to follow-up on in future PRs:
1. Further `arrow-avro/src/schema.rs` file refactoring (if possible). As I updated the file I saw other potential areas to improve. For example, I think if we introduced some sort of struct to hold context for `datatype_to_avro`, we could simplify and improve the logic.
Definitely future work.
2. Renaming `Nullability`. The name change will impact other areas of the code not related to the `Writer`. I'd also want to consider the impact `Union` type support will have as well when we get there. But I do think @scovich has a valid point about the current naming convention.
Yes. Mixing refactors with new functionality is always messy.

I would recommend two other follow-ups: 3. Profile-guided placement of #[inline] markers 4. Harmonize arg ordering for the various encoding methods that take out; some pass it as the first arg, others as the second arg.

@scovich All great call outs. I accepted your latest suggestions and pushed up changes that harmonized the arg ordering for the methods taking out as a parameter. As always thank you for the solid and thorough review 😄. We'll also be sure to follow-up on those PRs.

@alamb @mbrobbel Would either of you be able to look over this PR if you get a chance?

arrow-avro/src/writer/encoder.rs

Co-authored-by: Ryan Johnson <[email protected]>

arrow-avro/src/writer/encoder.rs

arrow-avro/src/schema.rs

Co-authored-by: Matthijs Brobbel <[email protected]>

alamb · 2025-09-08T15:44:13Z

I'll plan to merge this PR once the CI passes

alamb · 2025-09-08T16:41:02Z

Integration test failure is unrelated

alamb · 2025-09-08T16:41:27Z

Thanks @jecsand838

# Which issue does this PR close? - Part of #4886 - Related to #8274 # Rationale for this change This PR extends on work introduced in #8274, adding additional complex type support to the Avro writer. This brings us closer to a complete round-trip capability and Avro spec support in the arrow-avro crate. # What changes are included in this PR? New encoders: Fixed UUID IntervalMonthDayNano IntervalYearMonth IntervalDayTime Decimal32 Decimal64 Decimal128 Decimal256 Corresponding changes in support of these encoders in FieldEncoder and FieldPlan # Are these changes tested? Yes, additional complex type unit tests have been added. Benchmark tests have also been written but are being omitted here to keep the diff manageable. All tests, new and existing, pass. # Are there any user-facing changes? n/a, arrow-avro crate is not yet public

# Which issue does this PR close? - Part of #4886 - Related to #8274 and #8298 # Rationale for this change This PR adds Map and Enum encoders to the arrow-avro crate writer, along with new benchmark tests for remaining types and round-trip tests. # What changes are included in this PR? New encoders: **Map** **Enum** Corresponding changes in support of these encoders in FieldEncoder and FieldPlan ## Additional round trip tests in `mod.rs` New tests follow existing file read pattern - simple_fixed - duration_uuid - nonnullable.impala.avro - decimals - enum ## Additional benchmark tests for data types - Utf8 - List<Utf8> - Struct - FixedSizeBinary16 - UUID - IntervalMonthDayNanoDuration - Decimal32(bytes) - Decimal64(bytes) - Decimal128(bytes) - Decimal128(fixed16) - Decimal256(bytes) - Map - Enum # Are these changes tested? Yes, additional complex type unit tests have been added for Map and Enum. The rest of the PR beyond the new types are tests themselves. All tests, new and existing, pass. # Are there any user-facing changes? n/a, arrow-avro crate is not yet public --------- Co-authored-by: Connor Sanders <[email protected]> Co-authored-by: Andrew Lamb <[email protected]>

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

jecsand838 changed the title ~~Refactor arrow-avro writer to introduce unified RecordEncoder and s…~~ Added List and Struct Decoding to arrow-avro Writer Sep 3, 2025

jecsand838 commented Sep 3, 2025

View reviewed changes

jecsand838 changed the title ~~Added List and Struct Decoding to arrow-avro Writer~~ Added List and Struct Encoding to arrow-avro Writer Sep 3, 2025

nathaniel-d-ef reviewed Sep 3, 2025

View reviewed changes

Refactor arrow-avro writer to introduce unified RecordEncoder and s…

4e3950f

…implify field handling. Includes encoder type dispatch improvements, nullability support, and schema-driven write plans.

jecsand838 force-pushed the avro-writer-struct-list-types branch from d9ea385 to 4e3950f Compare September 3, 2025 17:44

scovich reviewed Sep 3, 2025

View reviewed changes

jecsand838 and others added 7 commits September 4, 2025 10:47

Update arrow-avro/src/schema.rs

fcb0853

Co-authored-by: Ryan Johnson <[email protected]>

Update arrow-avro/src/schema.rs

a73864d

Co-authored-by: Ryan Johnson <[email protected]>

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

83b15ed

Co-authored-by: Ryan Johnson <[email protected]>

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

0d3b3cd

Co-authored-by: Ryan Johnson <[email protected]>

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

8c9ab95

Co-authored-by: Ryan Johnson <[email protected]>

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

223b580

Co-authored-by: Ryan Johnson <[email protected]>

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

1ecbfce

Co-authored-by: Ryan Johnson <[email protected]>

jecsand838 force-pushed the avro-writer-struct-list-types branch from c388069 to 1ecbfce Compare September 4, 2025 17:39

Address PR Comments

960ad03

jecsand838 requested review from nathaniel-d-ef and scovich September 4, 2025 22:26

Merge branch 'main' into avro-writer-struct-list-types

7877b4f

scovich approved these changes Sep 5, 2025

View reviewed changes

jecsand838 and others added 6 commits September 5, 2025 13:05

Update arrow-avro/src/schema.rs

cafab61

Co-authored-by: Ryan Johnson <[email protected]>

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

f97d1af

Co-authored-by: Ryan Johnson <[email protected]>

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

3eae79c

Co-authored-by: Ryan Johnson <[email protected]>

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

47aaa18

Co-authored-by: Ryan Johnson <[email protected]>

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

971f26d

Co-authored-by: Ryan Johnson <[email protected]>

Update arrow-avro/src/schema.rs

82f4198

Co-authored-by: Ryan Johnson <[email protected]>

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

6f01bbc

Co-authored-by: Ryan Johnson <[email protected]>

jecsand838 force-pushed the avro-writer-struct-list-types branch from 7c8545b to 6f01bbc Compare September 5, 2025 18:28

jecsand838 added 2 commits September 5, 2025 13:47

Address PR Comments

a065386

Merge branch 'main' into avro-writer-struct-list-types

dad6533

Merge branch 'main' into avro-writer-struct-list-types

a3bde09

nathaniel-d-ef approved these changes Sep 5, 2025

View reviewed changes

scovich reviewed Sep 6, 2025

View reviewed changes

arrow-avro/src/writer/encoder.rs Outdated Show resolved Hide resolved

jecsand838 and others added 2 commits September 5, 2025 23:27

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

d0f0d9e

Co-authored-by: Ryan Johnson <[email protected]>

removed unused imports

90fc8b4

jecsand838 force-pushed the avro-writer-struct-list-types branch from 501666e to 90fc8b4 Compare September 6, 2025 04:56

Merge branch 'main' into avro-writer-struct-list-types

8dd0947

jecsand838 force-pushed the avro-writer-struct-list-types branch from 6733228 to 8dd0947 Compare September 7, 2025 19:44

mbrobbel approved these changes Sep 8, 2025

View reviewed changes

arrow-avro/src/writer/encoder.rs Outdated Show resolved Hide resolved

arrow-avro/src/schema.rs Outdated Show resolved Hide resolved

arrow-avro/src/schema.rs Outdated Show resolved Hide resolved

jecsand838 and others added 3 commits September 8, 2025 10:41

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

8a4d5c5

Co-authored-by: Matthijs Brobbel <[email protected]>

Update arrow-avro/src/schema.rs

7ec35c0

Co-authored-by: Matthijs Brobbel <[email protected]>

Update arrow-avro/src/schema.rs

6d96235

Co-authored-by: Matthijs Brobbel <[email protected]>

jecsand838 force-pushed the avro-writer-struct-list-types branch from 5585625 to df4d795 Compare September 8, 2025 15:43

Merge branch 'main' into avro-writer-struct-list-types

67f70d7

jecsand838 force-pushed the avro-writer-struct-list-types branch from df4d795 to 67f70d7 Compare September 8, 2025 16:09

alamb merged commit 911940f into apache:main Sep 8, 2025
22 of 23 checks passed

This was referenced Sep 8, 2025

Adds additional type support to arrow-avro writer #8298

Merged

Adds Map & Enum support to arrow-avro writer #8308

Closed

nathaniel-d-ef mentioned this pull request Sep 16, 2025

Adds Map & Enum support, round-trip & benchmark tests #8353

Merged

jecsand838 deleted the avro-writer-struct-list-types branch September 22, 2025 04:31

Added List and Struct Encoding to arrow-avro Writer #8274

Added List and Struct Encoding to arrow-avro Writer #8274

Uh oh!

Conversation

jecsand838 commented Sep 3, 2025

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!

Choose a reason for hiding this comment

Uh oh!

jecsand838 commented Sep 3, 2025

Uh oh!

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!

scovich left a comment

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!

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!

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!

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

Uh oh!

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!

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

Uh oh!

Uh oh!

scovich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scovich Sep 5, 2025 •

edited

Loading

jecsand838 commented Sep 4, 2025 •

edited

Loading