Store dateTime statements as Neo4j datetime values#812
Merged
Conversation
0b7e7e8 to
8fc0a9c
Compare
This was referenced Jul 4, 2026
Member
|
@malberts ready? |
Node properties built from subject statements were all written as plain scalars via `SET n = $props`, so a property whose Neo4j storage needs a Cypher constructor (datetime, date, point, duration) could not be stored in its native type and stayed un-queryable with the matching Cypher functions. Introduce Neo4jTypedValue, a carrier naming the Cypher constructor to wrap a value in. Neo4jSubjectUpdater now splits node properties into plain values and typed values: plain values keep going through `SET n = $props`, while each typed value is written with a parameterized, per-element constructor (`n.key = [v IN $param | <fn>(v)]`). This mirrors the existing Neo4jQueryStore::extractTypedValues pattern used for typed Page properties, adapted to multi-valued statement lists. No builder produces a Neo4jTypedValue yet, so behaviour is unchanged until a property type opts in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Statement values for the dateTime property type were written to Neo4j as plain ISO 8601 strings, leaving them un-queryable with Cypher's datetime functions (comparisons, ordering, range filtering). Register the dateTime value builder to return a Neo4jTypedValue wrapping the values in the `datetime` constructor, so SubjectUpdater stores them as native Neo4j datetimes. Un-skip DateTimeFormatNeo4jTest, which now asserts the stored property equals a list of datetimes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ters The Neo4j driver converts DateTimeInterface instances nested in the props map to native Bolt temporal values, so the dateTime builder can return DateTimeImmutable lists while the single-map SET n = $props write stays intact. This replaces the Neo4jTypedValue / Cypher-constructor mechanism, which required per-property query-string assembly and interpolated schema-defined property names into the query text. Values that are not strict ISO 8601 datetimes (DateTimeProperty's parse rules) are omitted from the graph projection instead of failing the whole write; the revision slot stays authoritative. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The result normalizer threw 'Unsupported Cypher value type' for every non-graph Cypher object, so any query returning a temporal value crashed the public query surface (reachable via RETURN datetime() and the typed page-side datetimes, and now hit by natively stored dateTime statements). Temporal values normalize to ISO 8601 strings, matching the shape statement values had when they were stored as strings. Other property objects (Duration, points, Vector) normalize to their array representation instead of throwing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
offsetDateTimeToIso reconstructed the instant from getSeconds() and shifted it with a whole-minute DateTimeZone, so sub-minute offsets (e.g. +00:19:32) lost their seconds in the wall clock and legacy Bolt-wire datetimes were offset twice. Derive the wall clock from toDateTime()->getTimestamp() plus the offset seconds via gmdate instead; the ISO 8601 suffix still truncates to whole minutes, which the format cannot express otherwise. Time and LocalTime previously fell through to their raw component maps, inconsistent with the ISO strings produced for the datetime siblings; they now render as HH:MM:SS(.f)(offset) too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The dateTime builder omits values it cannot parse, so an API/import write carrying a malformed value persists to the revision slot but silently diverges from the graph projection. Log a warning naming the property and page when a builder returns fewer values than it was given. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nces The cypher query surfaces now render temporal values as ISO 8601 strings and duration/point values as component objects, rather than rejecting them. Update the cell-normalization tables accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
410cceb to
850c112
Compare
This was referenced Jul 7, 2026
Contributor
Author
|
@JeroenDeDauw Ready after #1004. There are some minor follow-ups to do here, which CC got too excited to post already. |
* Render temporal values from raw Bolt fields in the result normalizer The Laudis client's toDateTime() builds its sub-second adjustment via DateTimeImmutable::modify() with a fractional-microsecond string, which PHP's relative-date parser mis-handles for certain sub-microsecond nanosecond values: 999999999 ns corrupted the year (rendered 9999) and 555555555 ns threw an uncaught DateMalformedStringException, while other values such as 1 ns and 123456789 ns rendered fine (there is no clean rule; the parser is simply erratic). Compute the wall clock from the raw seconds and offset instead, so the nanoseconds field only ever feeds the fractional-seconds renderer and every value is handled. NeoWiki's own writes are whole-microsecond (nanoseconds a multiple of 1000), so stored values were never affected; the bug reached arbitrary query results, such as datetime() literals with sub-microsecond precision or nanosecond-precision data written by other Bolt clients. Only the UTC-epoch (Bolt v5) wire encoding is handled; NeoWiki requires Neo4j 5.x, so the pre-Bolt-v5 local-epoch encoding cannot occur. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Cover offset-sign and non-UTC zone id rendering in the normalizer Mutation testing of the result normalizer surfaced two rendering paths the suite did not exercise: negative UTC offsets (the sign and abs handling in offsetFromSeconds) and DateTimeZoneId values whose zone is not UTC. Both already behaved correctly; these tests pin that behaviour so a later regression is caught. Unlike the fix commit's tests, these pass against the pre-fix code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
malberts
added a commit
that referenced
this pull request
Jul 10, 2026
The date value builder projected statement values into the graph as plain strings, so Cypher temporal operations on them silently misbehaved: comparing a string property to a date() value evaluates to null, which excludes the row without any error. PR #812 fixed this bug class for dateTime but left date behind. The builder now parses each value with DateProperty::parseStrictDate() and converts it to a Laudis Date (days since the Unix epoch), which the driver serializes as a native Neo4j DATE. A DateTimeImmutable would not do: the driver would store it as a DATETIME, keeping date comparisons cross-type. Unparseable values are dropped from the projection and reported through the existing dropped-value warning, matching the dateTime behavior. The read side already renders native dates as Y-m-d strings. Also unskips DateFormatNeo4jTest and extends it with normalization and date-comparison coverage mirroring DateTimeFormatNeo4jTest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
malberts
added a commit
that referenced
this pull request
Jul 10, 2026
The date value builder projected statement values into the graph as plain strings, so Cypher temporal operations on them silently misbehaved: comparing a string property to a date() value evaluates to null, which excludes the row without any error. PR #812 fixed this bug class for dateTime but left date behind. The builder now parses each value with DateProperty::parseStrictDate() and converts it to a Laudis Date (days since the Unix epoch), which the driver serializes as a native Neo4j DATE. A DateTimeImmutable would not do: the driver would store it as a DATETIME, keeping date comparisons cross-type. Unparseable values are dropped from the projection and reported through the existing dropped-value warning, matching the dateTime behavior; the shared scalar-parsing loop is extracted into a helper used by both temporal builders. The read side already renders native dates as Y-m-d strings. Existing graph data is re-typed by the standard rebuild (maintenance/RebuildGraphDatabases.php); until then, pre-existing string-stored date values silently don't match temporal comparisons. Also unskips DateFormatNeo4jTest, extends it with normalization and date-comparison coverage mirroring DateTimeFormatNeo4jTest, and updates the DemoData module comment that anticipated native date storage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
For #831
Statement values for the dateTime property type were written to Neo4j as
plain ISO 8601 strings, leaving them un-queryable with Cypher's datetime
functions (comparisons, ordering, range filtering).
Value builders registered with
Neo4jValueBuilderRegistrycan now returntyped objects: the Neo4j driver converts
DateTimeInterfaceinstancesnested in the props map to native Bolt temporal values, so the single-map
SET n = $propswrite stays intact and no Cypher constructor expressionsor query-string assembly are needed. The dateTime builder parses statement
strings with
DateTimeProperty::parseStrictDateTime(); malformed valuesare omitted from the graph projection (the revision slot stays
authoritative). Extension-defined property types get the same capability
by returning typed objects (e.g. Laudis
Date,Duration, points) fromtheir builders.
The result normalizer converts temporal values in query results to
ISO 8601 strings, and other Cypher property objects to arrays, instead of
throwing
Unsupported Cypher value type(previously reachable viaRETURN datetime()).Existing graph data is re-typed by the standard rebuild
(
maintenance/RebuildGraphDatabases.php); until then, pre-existingstring-stored values silently don't match temporal comparisons.
The page-side typed path (
PageValue/Neo4jQueryStore::extractTypedValues())is deliberately left untouched.
Follow-ups (out of scope, filed separately)
dateproperty type still stores plain strings (same un-queryability thisfixes for
dateTime): Storedatestatements as native Neo4j dates so date comparisons work #989non-UTC Neo4j timezone: Page-facts datetime path stores zone-less strings, shifting under a non-UTC Neo4j timezone #990