BigDecimal support via dbval's own tuple encoding - #16
Open
maxweber wants to merge 17 commits into
Open
Conversation
…ts BigDecimal The FoundationDB Java tuple layer has no BigDecimal type code and its encoder falls back to Number.longValue() for unknown Number subclasses, silently truncating BigDecimal values: a 0.5M discount was stored as 0. Since these tuples are only ever read by Clojure on the JVM, the cross-language constraint that kept FoundationDB's type set minimal does not apply. The new dbval.tuple namespace implements the encoding in Clojure: - Byte-compatible with the FoundationDB layer for every type it supported (nil, bytes, strings, nested tuples, integers including bignums, float, double, boolean, UUID), so existing stores stay readable. Verified generatively against fdb-java as a differential-testing oracle (now a :dev-only dependency). - Adds type code 0x23 for BigDecimal with an order-preserving encoding (sign marker, offset-binary adjusted exponent, mantissa digits), so index range scans over bigdec attributes work in numeric order. Values are canonicalized (trailing zeros stripped, integral values at scale 0) so numerically equal decimals encode identically. - Rejects unsupported value types (e.g. Ratio, unknown Number subclasses) with an exception instead of silently corrupting data. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ream Replace the FoundationDB tuple layer with an own encoding that supports BigDecimal
blob-key still called com.apple.foundationdb.tuple.Tuple/from, but PR #15 moved fdb-java to the root :dev alias, which a :local/root consumer never activates - (require 'dbval.store.slatedb) failed with ClassNotFoundException. dbval.tuple encodes strings and byte arrays byte-identically to the FoundationDB layer, so existing SlateDB files stay readable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
canonical-decimal called (.setScale d 0) on integral values, expanding the unscaled value before digit extraction: 1E+100000M became a 100,001-digit integer and a 100 KB index key (written 4x per datom), 1E+999999999M a ~1 GB key, and 1E+2147483647M an opaque ArithmeticException. The wire format already stores the adjusted exponent in its own int32 field, so encoding from the stripTrailingZeros form keeps the same canonical bytes at 8 bytes total - and restores the docstring's dn != 0 invariant that the setScale violated (500M encoded as digits "500" instead of "5"). Decoding still returns integral values at scale 0 (500 instead of 5E+2), but bounded to 100 restored zeros so reads never materialize a huge exponent either. Values whose adjusted exponent exceeds int32 are rejected with a clear error instead of wrapping the offset encoding. Ordering, canonicality, round-trip and byte-compat were property-tested (120k+ pairs incl. huge exponents) before landing; the generative specs now cover exponents up to 1E5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
The FoundationDB tuple spec reserves 0x23/0x24 for a future
arbitrary-precision decimal with a different encoding ('scale followed by
arbitrary precision integer', already used by layers), so an FDB tuple tool
would mis-decode dbval's incompatible 0x23 bytes instead of failing on an
unknown code. 0x40-0x4F is the range the spec sets aside for third-party
types. No released store contains decimal bytes yet (pre-PR BigDecimals
were truncated to longs), so the code can still move for free; cross-type
key order shifts, but no index semantics depend on it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
doubleToLongBits/floatToIntBits canonicalize every NaN, so a legacy store's NaN datom decoded and re-encoded (e.g. as a retraction key) produced different bytes than its stored assertion key, and the default computed NaN sorted above +Infinity where FDB sorted it below -Infinity. Raw bits keep byte compatibility for all payloads; the generative specs still exclude NaN (the round-trip property compares with =), so an explicit test pins the bit patterns against the fdb-java oracle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
String.getBytes replaces an unpaired surrogate (e.g. a caption truncated mid-emoji by subs) with '?', so "a\ud800b" and "a?b" encoded to identical index keys: reads returned a different string than was transacted, retracting one removed the other, and cardinality-many lost one of two distinct values. fdb-java rejected malformed UTF-16 loudly; the new utf8-bytes restores that policy for index keys and for the blob content hashes of deref attributes, which had the same aliasing through pr-str. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
The supported-type policy lived in three drifting conds (serialize-value,
serialize-tuple, dbval.tuple/write-value), and transacting an unsupported
value (e.g. a Ratio) surfaced as "set-add! failed" wrapping "pack
failed" wrapping "Unsupported tuple value type" - with the full datom
dumped into ex-data and logs, and no :error category or attribute.
dbval.tuple/supported-value? is now the single source of truth;
serialize-value and serialize-tuple validate against it and raise
:transact/unsupported-value-type with the attribute and a hint at
{:dbval/deref true} (write-value's throw stays as backstop). tuple-list
passes every :transact/* anomaly through unwrapped, so validation errors
never drag the offending datom into logs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
validate-inline-size only ran on the pr-str branch of serialize-value, so the raw decimal encoding (one byte per significant digit) could write index keys past the 60000-byte policy cap and SlateDB's 64 KiB key limit - the sqlite and memory stores accepted them silently, SlateDB failed at commit without attribute context. A digit-count bound on the stripped form raises the same :transact/value-too-large anomaly as oversized strings. This guard is still needed after the compact-encoding fix: it catches genuinely high-precision values, not just huge exponents. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
The byte encoder canonicalized scale representations (0.5M = 0.50M) but the two paths above it did not: deref attributes hashed the scale-sensitive pr-str, so (d/datoms db :eavt e attr 0.50M) missed a stored 0.5M and [:db/retract e attr 0.50M] silently no-oped on it - falsifying the 'same content <=> same hash' BlobRef invariant. And transact-add built the datom from the original-scale value, so :tx-data carried e.g. 19.90M while every later read returns 19.9M - tx listeners and audit trails recorded values that never round-trip. dbval.tuple/canonical-value returns the exact representative decoding produces; transact-add and value->blob-ref apply it, so index bytes, blob hashes, tx-data and reads agree on one value. Known pre-existing gap, unchanged here: the query engine joins a bound ?v input against deref attributes without hashing it, so [?e :attr ?v] with a raw bound value finds nothing regardless of scale; the datoms API and retraction work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
…ation The ns docstring claimed byte compatibility 'for all types that layer supports', but Versionstamps (never used by dbval) are rejected; the claim now scopes itself to the types dbval ever wrote. It also documents that stores written by the FoundationDB-backed code may still hold datoms whose BigDecimal / large-BigInt values were truncated to longs by the old Number.longValue() fallback - those decode as longs and cannot be matched or retracted by the original value, so upgrades need a scan-and-reassert. The README still credited the FoundationDB Tuple class with the tuple-to-byte-array logic; it now points at dbval.tuple. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
After the encoder rewire, dbval.db/tuple was a varargs alias for vec whose call sites only added copies: datom-tuple copied every 6-component tuple-list into a vector 4x per datom just for set-add! to pack it (tuple-codec/pack iterates any sequential), and tuple-range re-vec'd its components. datom-tuple thereby became identical to tuple-list, so the call sites use tuple-list directly; the query built-in 'tuple maps to clojure.core/vector, not this fn, so nothing else referenced it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
unescaped-length already proves whether a region contains escapes; when it does not (strings and hashes almost never contain NUL), read-escaped now uses one Arrays/copyOfRange instead of a byte-at-a-time loop, and the string decoder builds the String directly on the source array instead of through an intermediate byte array. unpack decodes at least two escaped regions per scanned row, so this is the read hot path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
pack runs four times per datom on every transact plus for every range bound, and it wrote through a synchronized BAOS one byte at a time - a ~40-byte key paid ~40 uncontended monitor operations, and write-escaped looped per byte even though strings essentially never contain NUL. The new ByteBuf deftype is a plain growable byte array, and write-escaped bulk-copies the chunks between NULs (a single arraycopy in the common case). Bytes are identical; the fdb-java differential specs and the generative ordering specs cover the rewrite. Packing a representative eavt key measures ~4x faster (5.3s -> 1.2-1.4s per 1M packs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
A bound ?v against a deref attribute found nothing: the pattern relation carried the datoms' BlobRefs, which never hash-join against the raw bound values (constant substitution only handled the search, not the join, and only for single-value bindings). Bound BigDecimals had the same problem across scale representations, because hash-join buckets by the scale-sensitive hashCode although = is scale-insensitive. lookup-pattern-db now detects these two cases - v bound in the context with a constant attribute that is deref (and at least one raw bound value), or with a BigDecimal among the bound values - and builds the relation by searching once per distinct bound value, binding the v column to that bound value so the join matches. All other shapes keep the scan-and-hash-join path, including deref-to-deref joins whose BlobRef columns already join correctly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
Three gaps in the bounded-index-key/clean-anomaly story: - An inline string with an unpaired surrogate was rejected only inside the byte encoder, surfacing as "set-add! failed" wrapping "pack failed" with the datom dumped into ex-data - the chain the unsupported-type validation had just eliminated. serialize-value now raises :transact/invalid-string with the attribute and char index. - Strings nested inside tuple values bypassed validate-inline-size entirely: [[:db/add e :pair [1 (70000-char string)]]] silently wrote a 70 KB index key. serialize-tuple now applies the same size and surrogate guards per component. - Components can each pass the per-value cap and still sum past SlateDB's 64 KiB key limit (e.g. a composite tuple of three 40 KB strings), which only failed at commit time on SlateDB and not at all on sqlite/memory. set-add! now checks the packed key against max-key-bytes and raises :transact/value-too-large with the attribute; like tuple-list, it passes :transact/* anomalies through unwrapped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
bound-v-vals built a distinct vector of all bound values before deciding whether the pattern needs the per-value lookup at all, adding a full pass with set allocations over the binding relation for every pattern whose v variable is bound - including the overwhelmingly common shapes that keep the default scan-and-hash-join path. The decision is now a streaming type check (a schema lookup plus at most one pass of instance checks); the distinct values are only materialized once the pushdown is chosen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf
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.
Replaces the FoundationDB tuple layer with dbval's own order-preserving encoding that supports BigDecimal (Tobias's #15, merged into this branch), plus the fixes from the deep review of that PR.
Review fixes on top of #15
Correctness
blob-keystill used the FDBTupleclass, whose dependency moved to the root:devalias that a:local/rootconsumer never activates. Now encoded viadbval.tuple(byte-identical, existing SlateDB files stay readable).canonical-decimalcalled.setScale 0, materializing trailing zeros —1E+100000Mbecame a 100 KB index key (written 4×/datom),1E+999999999M~1 GB (transactor OOM),1E+2147483647Man opaqueArithmeticException. Now encoded from thestripTrailingZerosform:1E+100000Mis 8 bytes and transacts in microseconds. Decode-side integer restoration is bounded, and adjusted exponents beyond int32 are rejected instead of wrapping the offset encoding.pr-str, so(d/datoms db :eavt e attr 0.50M)missed a stored0.5Mand[:db/retract e attr 0.50M]silently no-oped on it;:tx-datacarried19.90Mwhile every read returns19.9M.dbval.tuple/canonical-valueis now applied intransact-addandvalue->blob-ref, so index bytes, blob hashes, tx-data and reads agree.String.getBytessilently replaced them with?, aliasing distinct strings (e.g. a caption truncated mid-emoji) to identical keys and blob hashes. fdb-java's reject-loudly policy is restored.doubleToLongBitscanonicalized every NaN, breaking byte compatibility and re-encode idempotency for legacy NaN datoms.Hardening
max-inline-value-bytes; a digit-count bound now raises the same:transact/value-too-largeas oversized strings, protecting SlateDB's 64 KiB key limit.Ratiosurfaced asset-add! failed→pack failedwith the datom dumped into logs.dbval.tuple/supported-value?is the single source of truth;serialize-valueraises:transact/unsupported-value-typewith the attribute and a{:dbval/deref true}hint, andtuple-listpasses all:transact/*anomalies through unwrapped.Performance (hot paths: pack runs 4×/datom per transact + per range bound)
packwrites through an unsynchronized growable buffer with bulk-copied escape chunks instead of a synchronizedByteArrayOutputStreamone byte at a time: ~4× faster on a representative eavt key (5.3 s → 1.2-1.4 s per 1M packs).unpackbulk-copies escape-free regions and builds strings directly on the source array.tuplevec alias and the FDB-era per-datom vector copies (tuple-listoutput now feedspackdirectly).Post-review hardening (from a second audit pass)
:transact/invalid-stringwith the attribute at the serialization boundary instead of a wrappedpack failedchain with the datom in ex-data.validate-inline-sizeand silently write a 70 KB key).set-add!checks the packed key against a 64 KiBmax-key-bytesbackstop — components can each pass the per-value caps and still sum past SlateDB's key limit (e.g. a composite tuple of three 40 KB strings).Docs: byte-compat claim scoped to the types dbval ever wrote (Versionstamps excluded); documented that stores from the FDB era may hold BigDecimal/large-BigInt datoms truncated to longs by the old
Number.longValue()fallback (they need a scan-and-reassert on upgrade); README no longer credits the FDBTupleclass with the encoding.Verification
Query engine: bound values against deref attributes (pre-existing bug, also fixed here)
[?e :attr ?v]with a bound?vfound nothing for deref attributes — the pattern relation carried BlobRefs that never hash-join against raw bound values — and bound BigDecimals failed to join across scale representations, because hash-join buckets by the scale-sensitivehashCodewhile=is scale-insensitive.lookup-pattern-dbnow detects these two cases and looks the pattern up once per distinct bound value, binding the v column to the bound value so the join matches (also an index seek instead of an attribute scan). All other shapes keep the scan-and-hash-join path, including deref-to-deref joins, whose BlobRef columns already joined correctly.🤖 Generated with Claude Code
https://claude.ai/code/session_01UHpAtiY7bGbGJLYsYKdDvf