Merge erdomke fork improvements (d0f3634) with conflict resolution#4
Merged
Conversation
- Escaping backslashes in incoming Cypher is breaking string literals with escapes. - String values with escapes are not properly deserialized. - Values with the `::numeric` type annotation are not handled. - `Infinity`, `-Infinity`, and `NaN` are not properly handled. These are returned as raw tokens (not JSON strings). The current parsing also doesn't allow these values to be deeply nested in structures. - No handling for returning maps from Cypher - No way to dynamically handle the return type of a Cypher query at run time. The current code only works when the return types are known at design time. - No way to serialize/deserialize strongly-typed classes.
Incorporates fixes and improvements from erdomke's fork: - Generic Get<T>() deserialization with custom converters - Proper Infinity/NaN handling - Backslash escaping fix - ::numeric type annotations support - Edge<T>/Vertex<T>/Entity<T> records with polymorphism - Path as record with IReadOnlyList segments - Efficient byte-level I/O (ReadOnlySequence<byte>) - Dynamic return type support Resolved merge conflicts in Agtype.cs and updated tests for new API.
…hich doesn't exist on AGE 1.5.0
…ric type assertion, skip ::jsonb::agtype test on AGE 1.5.0
… InferredObjectConverter
…nverter, and test vertex array creation
…fix unit test assertions to match
There was a problem hiding this comment.
Pull request overview
This PR merges upstream fork changes to modernize Agtype parsing/serialization and graph entity modeling, aiming to improve correctness (Infinity/NaN, ::numeric, escaping) and performance (byte-level I/O, fewer allocations) while updating tests accordingly.
Changes:
- Refactors
Agtypeto store UTF-8 bytes (ReadOnlySequence<byte>) and introduces aGet<T>()pipeline based onJsonSerializer+ custom converters. - Introduces strongly-typed graph entity records (
Entity<T>,Vertex<T>,Edge<T>) and updatesPathto a record with segment lists. - Updates Npgsql extension methods and test suite to align with the new parsing/serialization behavior.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Npgsql.AgeTests/TestBase.cs | Adds AGE version detection to gate tests that require newer server features. |
| test/Npgsql.AgeTests/CypherHelperTests.cs | Removes the EscapeCypher backslash-escaping test (helper removed). |
| test/Npgsql.AgeTests/AgTypeTests.cs | Updates expectations for JSON-based parsing and new entity/path models. |
| test/Npgsql.AgeTests/AgeIntegrationTests.cs | Adds integration coverage for escaping, special values, and typed returns; updates array tests to list-based API. |
| src/Npgsql.Age/Types/Agtype.cs | Reworks Agtype storage/IO, adds Get<T>(), and implements agtype→JSON translation for deserialization. |
| src/Npgsql.Age/Types/Entity.cs | Adds base record type for graph entities. |
| src/Npgsql.Age/Types/Vertex.cs | Converts Vertex into record(s) using shared serialization options. |
| src/Npgsql.Age/Types/Edge.cs | Converts Edge into record(s) using shared serialization options and JSON property naming. |
| src/Npgsql.Age/Types/Path.cs | Converts Path into a record based on segment lists and serialization options. |
| src/Npgsql.Age/NpgsqlAgeExtensions.cs | Stops escaping cypher text and switches parameter serialization to Agtype.Create(...). |
| src/Npgsql.Age/Internal/CypherHelpers.cs | Removes EscapeCypher helper. |
| src/Npgsql.Age/Internal/AgtypeConverter.cs | Updates Npgsql type converter to read/write agtype using byte sequences and streaming writes. |
| src/Npgsql.Age/Internal/JsonConverters/SerializerOptions.cs | Splits read/write serializer options and adds converter/polymorphism configuration. |
| src/Npgsql.Age/Internal/JsonConverters/PathObjectConverter.cs | Implements custom (de)serialization for Path via $type + segments. |
| src/Npgsql.Age/Internal/JsonConverters/InferredObjectConverter.cs | Updates inference logic for object values and typed entities/paths. |
| src/Npgsql.Age/Internal/JsonConverters/EntityConverterFactory.cs | Adds a converter factory to emit ::vertex/::edge literals during serialization. |
| src/Npgsql.Age/Internal/JsonConverters/IntegerConverter.cs | Adds int converter that can parse special string-encoded numeric forms. |
| src/Npgsql.Age/Internal/JsonConverters/FloatConverter.cs | Adds float converter to write/read Infinity/NaN forms in agtype syntax. |
| src/Npgsql.Age/Internal/JsonConverters/DoubleConverter.cs | Adds double converter to write/read Infinity/NaN forms in agtype syntax. |
| src/Npgsql.Age/Internal/JsonConverters/DecimalConverter.cs | Adds decimal converter to emit and parse ::numeric literals. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…s private init, empty guard, XML doc fix, DecimalConverter invariant culture, remove Entity.GetHashCode, restore public Agtype ctor, fix DataSource leak, fix test reference
…ingConverter base)
Member
Author
|
fixes #5 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR merges the improvements from erdomke's fork into main, resolving merge conflicts.
Changes from the fork:
Get<T>()deserialization — central type-aware parser via JsonSerializer + custom converters\u0000prefix in ToJson, decoded by special converters::numerictype annotations — handled via ToJson → converter pipelineEdge<T>/Vertex<T>/Entity<T>records — strongly-typed graph entities with polymorphismPathas record — uses IReadOnlyList segmentsPreserved from existing code: