Skip to content

Refactors Apache.Arrow.Variant into two assemblies#322

Merged
CurtHagenlocher merged 4 commits into
apache:mainfrom
CurtHagenlocher:ScalarsPhaseOne
Apr 21, 2026
Merged

Refactors Apache.Arrow.Variant into two assemblies#322
CurtHagenlocher merged 4 commits into
apache:mainfrom
CurtHagenlocher:ScalarsPhaseOne

Conversation

@CurtHagenlocher
Copy link
Copy Markdown
Contributor

What's Changed

Refactors Apache.Arrow.Variant into two assemblies -- one for the base type and one for JSON conversion. The dependency on System.Text.Json moves to the second assembly.

…e type and one for JSON conversion. The dependency on System.Text.Json moves to the second assembly.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the Variant implementation by separating JSON-related functionality into a new Apache.Arrow.Operations assembly, allowing Apache.Arrow.Variant to drop its direct dependency on System.Text.Json while preserving JSON parsing/writing capability via the new assembly.

Changes:

  • Introduces Apache.Arrow.Operations (with Apache.Arrow.Operations.Json) to host JSON reader/writer/converter functionality.
  • Moves streaming value emission logic into a new VariantValueWriter, and updates VariantBuilder.Encode to use it.
  • Updates tests/benchmarks/solutions to reference the new assembly and namespaces.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/Apache.Arrow.Variant.Tests/VariantSqlDecimalTests.cs Updates JSON namespace usage to Apache.Arrow.Operations.Json.
test/Apache.Arrow.Variant.Tests/Apache.Arrow.Variant.Tests.csproj Adds project reference to Apache.Arrow.Operations.
test/Apache.Arrow.Variant.Benchmarks/EncodingBenchmarks.cs Switches streaming JSON encode benchmarks to VariantJsonReader.Parse.
test/Apache.Arrow.Variant.Benchmarks/Apache.Arrow.Variant.Benchmarks.csproj Adds project reference to Apache.Arrow.Operations.
test/Apache.Arrow.Operations.Tests/VariantJsonTests.cs Moves/renames JSON tests into the new operations test project + namespace.
test/Apache.Arrow.Operations.Tests/Apache.Arrow.Operations.Tests.csproj Adds new test project for Apache.Arrow.Operations.
src/Apache.Arrow.Variant/VariantValueWriter.cs Adds streaming writer for variant value bytes (used by builder and JSON reader).
src/Apache.Arrow.Variant/VariantBuilder.cs Refactors encoding to use VariantValueWriter; removes JSON encoding from this assembly.
src/Apache.Arrow.Variant/Properties/AssemblyInfo.cs Adds InternalsVisibleTo for Apache.Arrow.Operations.
src/Apache.Arrow.Variant/Json/VariantJsonReader.cs Removes old JSON reader from the Variant assembly.
src/Apache.Arrow.Variant/Apache.Arrow.Variant.csproj Removes System.Text.Json dependency.
src/Apache.Arrow.Operations/Properties/AssemblyInfo.cs Adds InternalsVisibleTo for operations tests.
src/Apache.Arrow.Operations/Json/VariantJsonWriter.cs Moves JSON writer into Apache.Arrow.Operations.Json and references Variant core types.
src/Apache.Arrow.Operations/Json/VariantJsonReader.cs Adds JSON-to-variant binary parser implemented with VariantValueWriter.
src/Apache.Arrow.Operations/Json/VariantJsonConverter.cs Moves JsonConverter<VariantValue> into operations assembly/namespace.
src/Apache.Arrow.Operations/Apache.Arrow.Operations.csproj Adds new assembly with System.Text.Json dependency and reference to Variant core.
Apache.Arrow.sln Adds new projects and solution folder structure (src/test), plus additional build configs.
Apache.Arrow.Tests.slnf Includes the new Apache.Arrow.Operations.Tests project in the test solution filter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Apache.Arrow.Variant/VariantValueWriter.cs
Comment thread src/Apache.Arrow.Variant/Properties/AssemblyInfo.cs
Comment thread Apache.Arrow.sln Outdated
Copy link
Copy Markdown
Contributor

@adamreeve adamreeve left a comment

Choose a reason for hiding this comment

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

Looks good to me, just some minor suggestions that you could choose to ignore

Comment thread test/Apache.Arrow.Variant.Tests/VariantSqlDecimalTests.cs Outdated
Comment thread src/Apache.Arrow.Variant/Properties/AssemblyInfo.cs
@CurtHagenlocher CurtHagenlocher merged commit a25e49b into apache:main Apr 21, 2026
14 checks passed
@CurtHagenlocher CurtHagenlocher deleted the ScalarsPhaseOne branch April 23, 2026 00:03
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.

3 participants