feat: TIME column type with granularity qualifier (#43) - #47
Merged
Conversation
Add TIME/TIME(n) as a first-class CREATE TABLE column type, stored as HH:MM text. The optional TIME(n) qualifier requires minutes to be a multiple of n (e.g. TIME(15) for quarter-hour increments). A new ColumnMetaStore (data/system.sqlite3, same pattern as IndexStore) tracks base type + qualifier per column since SQLite's own type affinity can't distinguish TIME from CHAR/DATE (all TEXT). REPLACE ... WITH validates against it and rejects malformed or off-granularity values instead of silently coercing them; LIST STRUCTURE prints the declared type. The New table wizard and Modify structure wizard both offer TIME as a column type.
Closed
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.
Closes #43.
Summary
TIMEcolumn type (CREATE TABLE ... col TIME), storageHH:MMtext, same family asDATE/CHAR.TIME(n)(e.g.TIME(15)for quarter-hour increments) — parsing was already free via the existingCHAR(n)/NUM(p,s)grammar.ColumnMetaStore(data/system.sqlite3, mirrorsIndexStore's shape) tracks base type + qualifier per column, since SQLite's own type affinity can't distinguishTIMEfromCHAR/DATE(all map toTEXT).REPLACE ... WITHvalidates against the column'sTIME/TIME(n)metadata and rejects malformed or off-granularity values with a clear** Error: ...— no silent coercion.APPEND RECORDdefaults are alwaysNULL, which is left unvalidated (matches existing behavior for every other type).LIST STRUCTUREprints the declared type (TIME,TIME(15)) instead of the raw SQLite storage class for columns tracked inColumnMetaStore.DROP TABLE/ALTER TABLE DROP/RENAME/ALTERkeep the metadata consistent (drop/rename/clear as appropriate).TIMEwith an optional granularity field; Modify structure offersTIMEas a retype target.Scope notes
ALTER TABLE ADD/ALTER ... TIME(n)does not carry the qualifier through (existingskipTypeSize()behavior for all types, e.g.CHAR(n)size is already ignored onALTER TABLE— consistent, not a regression). OnlyCREATE TABLEcaptures the qualifier. Noting this explicitly per the issue's guidance to confirm/extend only if needed — full ALTER TABLE qualifier support wasn't requested and adds parser/executor surface without a driving use case yet.ModStructWizard's existingw3type()detection (raw SQLite type → picker default) can't detect an existingTIMEcolumn and will default it toCHAR— pre-existing limitation of that heuristic, not new.Test plan
npm test— 272/272 vitest (18 files), including newtests/TimeType.test.ts(7 cases: creation, structure listing, valid/invalid/out-of-range REPLACE, granularity accept/reject, NULL-safe APPEND).npx playwright test— 74/74 (was 73), new case intests/assistant.spec.tsdrives the New table wizard end-to-end (TIME(15)column,LIST STRUCTURE, reject08:07, accept08:15,LISTshows the committed value).npx tsc --noEmitclean.