fix(q): convert minute/second/month units on decode; null out-of-range datetime - #7
Open
belowzeroff wants to merge 1 commit into
Open
belowzeroff wants to merge 1 commit into
belowzeroff wants to merge 1 commit into
Conversation
…e datetime The decoder copied every 4-byte q temporal raw and re-tagged it as the rayforce type of the same width, which is only correct for date (KD) and time (KT). minute (KU) counts minutes and second (KV) counts seconds since midnight, but both landed in TIME (i32 milliseconds) unscaled — 09:30 came back as 00:00:00.570 and 09:30:15 as 00:00:34.215. month (KM) counts months since 2000.01 but landed in DATE (i32 days) — 2024.02m came back as 2000.10.16. Every result carried the correct type code, so nothing downstream could tell the values were wrong. Decode KU and KV by scaling to ms, and KM to the DATE of the month's first day (what q's `date$ does with a month). Values the target type cannot hold — the q infinities 0W/-0W, a minute/second count past the i32 ms range — decode to the typed null, and the null sentinels map to the null as before. The datetime (KZ) conversion had the mirror problem at the edges: it fed any double through llround and an i64 multiply, so a datetime past the TIMESTAMP nanosecond range (or 0Wz) overflowed into a plausible-looking timestamp. Range-check the day count and decode those to the null timestamp. timespan (KN) is unchanged and still decodes to a TIMESTAMP anchored at 2000.01.01: rayforce has no duration type, and that mapping is what kdb-tick style `time` columns rely on today. Tests: 05_temporal.rfl covers atoms, vectors, nulls, infinities and the null gate on aggregates against a real q; the codec selftest checks the raw wire bytes so the conversion is also verified where no q is available.
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
1 participant
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.
How it looks from the user's side
Pull anything with a q
minute,secondormonthcolumn through.q.send— aselect by 5 xbar time.minutebucket, asecond-typed timestamp, a monthly key — and the values come back as the right type with the wrong value, silently:09:30(minute)00:00:00.57009:30:00.00009:30:15(second)00:00:34.21509:30:15.0002024.02m(month)2000.10.162024.02.01Nothing errors; every result carries the correct TIME/DATE type code, so joins, bucketing and filters downstream just work on garbage.
A datetime (
KZ) past the TIMESTAMP nanosecond range, or the q infinities0Wz/-0Wz, had the mirror problem at the edges: an overflowed but plausible-looking timestamp instead of a null.Root cause
The decoder copied every 4-byte q temporal raw and re-tagged it as the rayforce type of the same width. That is right for
date(KD → DATE, days) andtime(KT → TIME, ms), and wrong for the other three sharing the width:minute(KU) counts minutes andsecond(KV) counts seconds since midnight, but landed in TIME's i32 milliseconds unscaled;month(KM) counts months since 2000.01 but landed in DATE's i32 days.q_kz_days_to_nanosfed any double throughllroundand an i64 multiply with no range check.Fix
KU→ TIME scaled ×60000,KV→ TIME scaled ×1000, atoms and vectors.KM→ DATE of the month's first day, what q's`date$does with a month (2024.02m→2024.02.01).0W/-0W, a minute/second count past the i32 ms range, a datetime past the TIMESTAMP range — decodes to the typed null rather than wrapping; the null sentinels (0Nu/0Nv/0Nm/0Nz) map to the null as before, and the converted vectors carry theHAS_NULLSgate so aggregates skip them.timespan(KN) is deliberately unchanged: rayforce has no duration type, and its current mapping to a TIMESTAMP anchored at2000.01.01is what kdb-tick styletimecolumns rely on today. Worth a separate decision.Tests
test/rfl/client/05_temporal.rfl(real q, both the blocking and--pollruns): minute/second/month atoms and vectors, negative minutes, nulls inside vectors, infinities → null,`date$2024.02magreement, andmin/maxover vectors with nulls (the null gate).test/driver.ccodec selftest: the raw wire bytes for a minute atom, a second atom,0Wu, a month atom, a negative month (1999.12m) and an out-of-range datetime — so the conversion is verified in CI, where no q is available. All six fail on the previous decoder.make testagainst currentdevcore, with a local q 4.x: codec + exchange selftests ok, server 33/33, real-q interop 30/30, client 91 assertions × 2 runs + push 27, 0 failures.