feat(sdk): shared timestamp-axis policy header (0.27.0) - #187
Merged
Conversation
Five official plugins each carry their own rule for choosing a topic's time axis, with four different name lists and plausibility rules and no test that any two agree: the same Parquet file gets a different axis through the parquet loader than through arrow-ipc, an int8 column named `ts` could silently become an axis, and two plugins carried verbatim copies of each other's seconds-to- nanoseconds conversion that have since diverged (#186). pj_plugins/sdk/timestamp_policy.hpp is the one contract, in the shape of parser_array_policy.hpp: a native-timestamp type pass, then a name pass over the union of every list in use, restricted to storage that can actually hold an epoch nanosecond (TIMESTAMP, int64, uint64, double) on scalar leaves; an explicit narrow-integer, uint32 or float32 axis is accepted with a warning the plugin surfaces; secondsToNanoseconds is integer-split so every plugin and every platform computes the same nanosecond; and timestamp_column / timestamp_unit are canonical config keys so the unit stops being inferred from storage type. Header-only, additive, no ABI change. Adoption in parser_arrow, toolbox_mosaico, data_load_parquet and data_load_lerobot is separate work in pj-official-plugins.
…policy header keeps only policy The absolute time spine (pj_base/time.hpp) forbade hand-rolled 1e9 conversions in its own comment and then provided none, so the host, the parser modules and every plugin wrote their own. The first cut of this branch added the arithmetic to the plugin policy header, which the host cannot include; and the spine itself is C++20 (sys_time), while parser-module headers are held to C++17 by pj_parser_module's cxx_std_17 and a -std=c++17 compile check. pj_base/time_math.hpp is the C++17-clean home: TimeUnit and nanosecondsPer, checked scaleToNanoseconds, widenUnsignedTicks, the integer-split secondsToNanoseconds, combineSecondsAndNanos, syntheticInstant, fitSyntheticInterval and kDefaultSyntheticIntervalNs, all constexpr/inline with std::optional on overflow. time.hpp includes it so spine users see one PJ surface; parser_module/time.hpp forwards its combine to it with error texts unchanged; the new header joins the C++17 compile check. pj_plugins/sdk/timestamp_policy.hpp keeps detection, axis support and the config keys, now typed on PJ::TimeUnit, and adds synthetic_interval_ns and flatten_structs as canonical keys. Header-only, no ABI change; abi/baseline.abi byte-identical.
detectTimestampColumn fuses two questions: does this name look like an axis, and is this type usable as one. A transport that frames a schema for a downstream parser needs the first question alone: when nothing plausible survives framing but the raw schema carried a leaf named like an axis, it must fail loudly rather than run on a synthetic axis, and it must ask that without re-implementing the name list privately. matchesTimestampName(name, policy) returns the priority index of the first policy name matched, exact-case first and ASCII case-folded when the policy allows, or nullopt. detectTimestampColumn's name pass is rebuilt on it with its tie rule unchanged.
…igured unit timestampEligibility(storage, unit) replaces the unit-blind axisSupport(kind): 32-bit integers are eligible for automatic selection when a tick is a second, 8/16-bit integers and float32 are explicit-only at every unit, 64-bit storage always qualifies. TimestampPolicy carries the unit so detectTimestampColumn and the warning text agree with the timestamp_unit key instead of assuming integers are nanoseconds. Renames while nothing consumes the header yet: TimeKind -> TimestampStorage, AxisSupport -> TimestampEligibility, matchesTimestampName -> timestampNamePriority, widenUnsignedTicks -> toSignedTicks. Co-Authored-By: Claude Fable 5.1 <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
2 participants
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.
Closes #186.
What — two layers
pj_base/time_math.hpp(new, C++17-clean, namespacePJ, re-exported by the C++20 spinepj_base/time.hpp) — the checked integer time arithmetic the spine's own header comment forbade hand-rolling but never provided, so the host (arrow_import.cpp), the parser modules (parser_module/time.hpp, which now forwards to it) and every plugin can share one implementation:TimeUnit+nanosecondsPer,scaleToNanoseconds,toSignedTicks,secondsToNanoseconds(integer-split, platform-independent — replaces thelong doublecopies two plugins carried and had already let diverge),combineSecondsAndNanos,syntheticInstant,fitSyntheticInterval,kDefaultSyntheticIntervalNs. Allconstexpr/inline,std::optionalon overflow, no exceptions, no allocation.pj_plugins/sdk/timestamp_policy.hpp— the plugin-facing policy layered on it, in the shape ofparser_array_policy.hpp:detectTimestampColumn(candidates, policy)— type pass first (the first scalar native TIMESTAMP), then the name pass in priority order overkCanonicalTimestampNames(the union of every list the official plugins use today, most specific first), restricted tokEligiblestorage on scalar leaves. Case-insensitive by default, exact-case match wins a tie, list elements are never auto-selected.timestampEligibility(storage, unit)/explicitOnlyWarning(storage, unit)— judges a column'sTimestampStorageagainst the configuredtimestamp_unitinstead of assuming integers are nanoseconds: 64-bit integers, native timestamps anddoubleare alwayskEligible; 32-bit integers only when a tick is a second (int32 reaches 2038, uint32 2106); 8/16-bit integers andfloat32arekExplicitOnlyat every unit — honored when a producer names them, with the sentence the plugin must surface. Everything else iskIneligible.TimestampPolicycarries the unit so the detector agrees with the config.timestampNamePriority(name, policy)— the name pass on its own (priority index into the policy's names, exact-case first, then ASCII case-folded), for plugins that name the column explicitly.timestamp_column,timestamp_unit(ns|us|ms|s, withtimestampUnitFromJson/ToJsontyped onPJ::TimeUnit),synthetic_interval_nsandflatten_structs, so producers and consumers name the whole axis contract the same way and the unit stops being inferred from storage type.Why
Five plugins carry their own detector with four different rules and no test that any two agree (inventory in #186). The same Parquet file gets a different axis through the parquet loader than through
arrow-ipc; anint8column namedtscould silently become a topic's time axis; and two plugins carried verbatim copies of each other's seconds→ns conversion that have since diverged.Versioning
MINOR:
0.26.0→0.27.0. Header-only, additive, no ABI change;abi/baseline.abibyte-identical.time_math.hppis verified to compile under-std=c++17because parser-module headers require it.Adoption (separate PRs in pj-official-plugins)
parser_arrow(#279),toolbox_mosaico(#282),data_load_parquet,data_load_lerobot. Notdata_load_csv(untyped text, user-selected column) ordata_load_ulog(format-mandated field).