Skip to content

Multi-DB support - #208

Open
joshuajaharwood wants to merge 27 commits into
planarnetwork:masterfrom
velocirail:multi-db-kysely
Open

joshuajaharwood wants to merge 27 commits into
planarnetwork:masterfrom
velocirail:multi-db-kysely

Conversation

@joshuajaharwood

@joshuajaharwood joshuajaharwood commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Multi-DB support

Runs the import and the GTFS build against Postgres and SQLite as well as MySQL.
DATABASE_DIALECT picks one and defaults to mysql, so an existing install is
unaffected.

What changed

  • Tables are declared in apps/dtd2mysql/src/database/schema rather than implied
    by the feed definitions. Schema.spec checks the declarations against
    @gb-transit/dtd-schema: every table all four feeds write to is described, with no
    column narrowed, renamed or reordered.
  • Schema and statements go through Kysely, so one declaration produces a MySQL,
    Postgres or SQLite table. SchemaDialect holds the three things Kysely does not
    cover: the column type per field, the surrogate key, and the duplicate-index error.
  • --fares-clean, --gtfs-import, the feed cursor and the GTFS build all follow.
    MySqlTimetableSource is unchanged and still serves MySQL — it streams through
    mysql2 directly, which is worth keeping for a full feed's stop times.
  • Four MySQL-only spellings in the GTFS queries are now standard SQL: <=>, IF,
    a fixed link matched on concatenated codes, and a GROUP BY naming fewer columns
    than it selects. A char column reads the same everywhere: MySQL strips the
    padding, Postgres pads it back, so it is dropped when the field is read. MySQL's
    rows do not move.
  • Connection configuration belongs to the consumer: DATABASE_URL and
    DATABASE_OPTIONS (JSON) are handed to the driver as-is, so anything the driver
    supports is reachable — a unix socket, TLS, pool sizing — without this code knowing
    the option exists. The dialect comes from the URL scheme. dateStrings and the pg
    date/timestamp parsers are applied over the top: the declared types say a date is a
    string, and reading one should not depend on the reader's timezone.
  • SQLite uses Kysely's own SqliteDialect over node:sqlite, adapted in ~45 lines.
    No native module, so a whole feed imports and queries with no server.
  • pg and pg-cursor are optional peer dependencies.

Fixes

The orphan stop-time cleanup ran after every feed and named three timetable tables,
so importing a single feed into a database that had never held a timetable failed on
a table that was never created. Invisible when all four feeds share one database.

Testing

CI gains a database matrix: the import runs against MySQL, Postgres and SQLite, with
every row compared to the same recorded expectations whichever answered — a database
needing its own expectations is a bug rather than something to record. Unit tests run
on Node 22 and 26, both sides of the Temporal polyfill.

Locally against MariaDB 10.11, Postgres 16.13 and node:sqlite: integration suite
91 tests each, and scripts/check-database.mjs green on all three — feed built from
the database byte-identical to the feed built from the files, and a second import
changing nothing.

@joshuajaharwood
joshuajaharwood force-pushed the multi-db-kysely branch 2 times, most recently from 7d4bf67 to 5e731d2 Compare September 14, 2026 18:12
The first slice of the multi-database work, re-homed onto the monorepo. The
tables are declared in apps/dtd2mysql/src/database/schema rather than being
implied by the feed definitions, and SchemaBuilder creates them through Kysely
so the same declaration produces a MySQL, Postgres or SQLite table.

The declarations live with the app rather than in libs/dtd-schema on purpose.
That package says where a value sits in a DTD record and how to read it, which
is a different question from what the database holds, and only dtd2mysql asks
the second one.

SchemaDialect is the part Kysely does not cover. Its own Dialect decides how to
talk to a database - driver, placeholders, quoting - and passes column types
through to the compiler, so something still has to say that a DoubleField is an
unsigned double in MySQL and a double precision in Postgres. Three of those are
declared here, along with the surrogate key each database spells differently and
the error a duplicate index raises.

Schema.spec checks the declarations against the feed definitions in
@gb-transit/dtd-schema, which is what says the port still fits upstream: every
table all four feeds write to is described, with no column narrowed, renamed or
reordered.

The test helpers sit under src/database/testing and are excluded from the build
rather than shipped in dist, since the package tsconfig only excludes specs.

MySQLSchema and MySQLTable are untouched for now; the container still wires
them, and they come out when the writer lands.
A char column does not mean the same thing everywhere. MySQL strips trailing
spaces on the way out, Postgres pads the value back out to the width of the
column, and SQLite returns exactly what it was given, so the same feed produces
three different strings.

The padding is how a fixed width line reaches its next field rather than part
of the value, so TextField drops it when the field is read and it never reaches
a database. MySQL was already stripping it, so its rows do not move.
VariableLengthText keeps its trailing spaces: nothing padded those out, so they
are the value.

ForeignKeyField takes a structural type rather than the two record classes. It
only ever reads lastId, and naming the classes made their field maps have to
match exactly. RecordWithLastId stays internal to the package rather than being
re-exported, so the committed public surface is unchanged.
MySQLTable and MySQLSchema wrote MySQL by hand: INSERT IGNORE, REPLACE and a
CREATE TABLE built out of the feed definitions. TableWriter and SchemaBuilder
replace them, so the same import runs against MySQL, Postgres or SQLite,
chosen with DATABASE_DIALECT and defaulting to MySQL.

Leaving an existing row alone is spelled differently by each database and
Kysely covers all three. Replacing one is not: MySQL's REPLACE and SQLite's
INSERT OR REPLACE delete what clashes and insert a new row with a new id,
while Postgres only has ON CONFLICT DO UPDATE, which keeps the row and its
id. The delete and the insert are spelled out inside a transaction so every
database ends up with the same rows.

Rows are inserted by name rather than position, as VALUES ? was a mysql2
feature, and the id is left out when the database generates it because
Postgres will not take a null there. A flush is split so no statement binds
more values than Postgres or SQLite accepts, and ordered inserts chain their
flushes rather than holding a connection open.

Two things upstream had already fixed are kept rather than reverted to what
the original branch did. restoreIdCounters covers every record with a counter
where the old code only restored the schedule one, and the orphan clean up
covers z_stop_time; both are expressed through Kysely here. The table name in
restoreIdCounters is only known at runtime, so it is one of the two places
that cannot be checked against the Database type.

The drivers are optional peer dependencies, loaded on the dialect that needs
them. SQLite needs nothing, it is built into node.
MySqlTimetableSource stays as it is and keeps MySQL: it streams through mysql2
directly, which is worth having for the millions of stop time rows a full feed
has. KyselyTimetableSource is the same five questions asked through the query
builder, so Postgres and SQLite can answer them too, and the container picks
one on the dialect.

Four things in the MySQL version do not translate, and each has a test:

- cate_interchange_status <=> 9 is MySQL's null safe equals. A CASE says the
  same thing everywhere, including for the null the operator quietly treats as
  "not 9", which a plain = would drop.
- IF(train_status = "S", ...) is a MySQL function. CASE is the standard
  spelling, and the ship it marks is a ferry rather than rail in the output.
- a fixed link matched on CONCAT(origin, destination) is two codes compared
  glued together, so one pair can suppress another. Three character codes
  cannot collide, but a shorter one occurs once the fixed width padding is
  stripped, so the match is on the pair.
- a bare GROUP BY with columns that are not in it. MySQL answers by choosing a
  row for you, Postgres rejects it outright. The row is chosen here, keeping
  upstream's preference for the station over the junction that shares its CRS.

ScheduleBuilder gains loadStream alongside loadSchedules, because a query
builder yields rows where a driver emits them. The emitter path is untouched,
so the MySQL source and the file source are unaffected.

The queries are built rather than written, so they are checked against the
schema declarations rather than being strings that happen to name columns.
Every statement is dialect neutral, so the specs run against a real SQLite
database and the integration suite is what covers the other two.
…tabase

The three commands still speaking MySQL directly.

The GTFS import shelled out to the mysql client with LOAD DATA LOCAL INFILE,
which wanted a MySQL server, the mysql binary on the path and the password on
a command line. The tables are declared rather than written out as DDL, and
the files are read here. Three things fall out of that: the columns are
matched by the file's header rather than by a position list, so stops.txt
writing its coordinates last no longer loads them into zone_id and stop_url;
the values are quoted CSV rather than split on every comma, so a station name
with one in it survives; and an absent value is nothing rather than a zero or
a blank date.

The declarations are taken from the schema this branch is built on rather than
from the one the original work started from, so they carry the attributions
and feed_info tables it has gained and not the links table it has dropped. A
spec checks them against the golden feed dtd2gtfs commits, in place of the one
that checked the positional column lists, and the import itself now has a test
because it no longer needs a server to run.

The fares clean up. CURDATE() is worked out in JS, so what has expired no
longer depends on the machine running it and can be pinned in a test.
Recording which restriction applies to an origin, destination and route was a
GROUP BY that left direction and restriction_code off it, which MySQL answers
by picking a row for you and Postgres rejects, so the fare the values are
taken from is chosen here instead. The steps ran at the same time as each
other, which left it open whether a fare had been removed before or after the
restrictions were worked out from it; they run in order now, which also takes
away the deadlocks they retried through. network_flow_restriction was a CREATE
TABLE inside the command and is declared with everything else.

The download path needed nothing: upstream had already lifted the one query it
makes into FeedCursor, so only LogTableFeedCursor moved to Kysely.
@joshuajaharwood
joshuajaharwood marked this pull request as ready for review September 17, 2026 17:15
The suite that says the port is right rather than that it compiles. Every row
of every table the four feeds write to is recorded, and the same file is the
expectation for all three databases: one needing its own is a bug rather than
something to record.

The fixtures are both kinds. The timetable and the fares locations come from
the mini feeds dtd2gtfs already commits, which are slices of real refreshes -
7,336 MCA lines seeded from named TUIDs, and the LOC records the station
groups extension reads. Fares, routeing and nfm64 are generated from the field
definitions, because nineteen files and forty five tables against the DTD spec
is not something to transcribe by hand. The generated ones say the database
layer returns what the feed parsed; the real ones say real records parse, and
BEDFORD+BUS arrives without the padding that fills its field out.

Recorded as one .tsv per table rather than as JSON. JSON repeats every column
name on every row, which was 3.7 MB against 416 KB for the same rows, and the
repo already writes its database fingerprints as .tsv. One file per table means
a change to stop_time is not also a diff against everything else.

The declared schema is checked against data/snapshots/db-all-feeds, which was
cut from a real database the four feeds had been imported into: 79 tables, same
columns, same order. Only the column list, not the row hashes - the README is
explicit that those are specific to the machine that cut them.

The database job becomes a matrix over the three dialects. It already imported
the mini fixture and diffed the feed built from the database against the one
built from the files; doing that per dialect is what holds the new
KyselyTimetableSource to what the MySQL one produces.

pg and pg-cursor are optional peer dependencies, installed by somebody pointing
at Postgres. mysql2 stays a plain dependency, so an existing install is
unaffected and this needs no major version.
Two tables came back empty: alias and toc_interchange. Neither is a bug in the
import, and neither was noticed when the fixture changed.

RJTTF001.ZIP is a slice of a real refresh, and a slice only holds what its seed
TUIDs pulled in. It has no TSI file at all, which is the timetable feed's only
CSV record rather than a fixed width one, so nothing exercised that parser end
to end. Its MSN is 205 lines and every one of them is an A record, so the L
records - station aliases, the file's second record type - were not covered
either.

The hand built fixture covers both, the same way the generated fares fixture
covers the tables the real LOC slice does not reach. Four stations, one alias
and two interchange rows, which is enough to say the records parse and store.

Four recorded files are still empty and are meant to be: the two tables the
real fares slice has no records for, which the generated fixture covers, and
the two here, which the real timetable slice has no records for. An empty file
still asserts the table is empty, so a regression that started writing rows
would fail rather than pass quietly.
Importing the fares feed into a database that has never had a timetable feed
failed on "no such table: stop_time". The orphan clean up runs after every
import and names three timetable tables, but only the feed being imported has
had its tables created.

Invisible where all four feeds share one database, which is how anyone runs
this, so nothing had ever hit it. It shows up the moment a feed is imported on
its own - which is what a fresh in-memory database is.

The command already knows which tables its feed declares, so it asks.
Tidy up the loose ends the port left.

The README still said only MySQL was supported and pointed at a PR that would
change it. It now has a configuration section: the environment variables the
commands read, which driver each database needs, and the SQLite example, which
is the one that needs no server at all.

The contributing notes say how to run the integration suite and where its
recorded rows live, beside the golden feed paragraph that already says the same
for the GTFS output.

docker-compose gains the Postgres service CI already had, so the suite can be
run against all three locally rather than only the two.

rowOf was ported for a fares clean up spec that is not in this branch and
nothing else calls it, so it goes rather than sitting there looking used. The
dialect constant is only read where the SQLite default is chosen, so it is no
longer exported.

And the changeset: this reaches users, so it needs one.
Two things still spoke only MySQL to a user.

The help text said the tool imports into "a MySQL compatible database", named
the environment variables as mysql ones and did not mention DATABASE_DIALECT at
all, so the only place the other two databases were documented was the README.

DATABASE_PORT defaulted to 3306 whatever the dialect, so pointing at Postgres
without setting it dialled the MySQL port and failed on connection refused. It
now defaults to the port the dialect uses.

What is left that names MySQL is either deliberate - MySqlTimetableSource keeps
its own streaming path - or a comment saying which database behaves how, which
is the thing worth writing down.
A patch release. 0.30 is beta only, so this is the current stable.

Nothing moved: the schema and writer specs assert the compiled SQL string by
string, so a change in what Kysely generates would fail there rather than
quietly, and the recorded rows and the golden feed are both unchanged.
Kysely models no connection configuration of its own. Every dialect takes the
driver's object - a mysql2 pool, a pg pool, a node:sqlite handle, tedious and
tarn for MSSQL - and says nothing about how it was built. This follows it.

DatabaseConfiguration was mysql2's shape under a neutral name. It carried
multipleStatements and dateStrings, which only mysql2 has, and connectionLimit,
which pg calls max. MySQL took it through a double cast and Postgres
destructured six fields and dropped the rest, so anything a driver supported
beyond those six could not be asked for. A unix socket was the example:
socketPath is a mysql2 option and a host naming a directory is a pg one, and
neither had anywhere to go.

DATABASE_URL is handed to the driver as it is, and DATABASE_OPTIONS is JSON
merged into whatever the driver is given. Both are passthrough, so the driver's
surface is the surface, and an option it gains later needs no change here.

The dialect comes from the URL's scheme, so postgresql:// needs no
DATABASE_DIALECT beside it. A scheme with no dialect is an error rather than a
fall back to MySQL, which would otherwise hand a Postgres URL to mysql2 and fail
somewhere much less obvious. The scheme is read with a pattern rather than URL,
which rejects the hostless form a socket connection uses.

SQLite has no server to address, so a URL only names its file, and the options
node:sqlite takes are reachable the same way as the others.

dateStrings and the Postgres date and timestamp parsers are applied over
whatever is given. They are not preferences: the declared column types say a
date column is a string, and reading one should not depend on the timezone of
the machine that reads it.
The driver here reimplemented what Kysely already ships: a connection, the
transaction commands, the streaming and a mutex. Its SQLite dialect does not
depend on better-sqlite3 - SqliteDatabase is a structural interface of close()
and prepare() - so node:sqlite only needs adapting to it, not replacing it.

Three differences, which is all that is left: better-sqlite3 binds parameters
from an array where node:sqlite takes arguments, it says whether a statement
returns rows with reader where node:sqlite has columns(), and node:sqlite binds
a narrower set of values than the query builder produces.

Savepoints come for free, which the driver here did not have.

The community node:sqlite dialect was the other way to stop owning this, and it
is not taken because its streamQuery runs the query and yields every row as one
chunk rather than iterating. The GTFS build reads the stop times of a whole feed
through that path, which is the reason it streams at all. It also decides
whether a statement returns rows by looking for "returning" in the SQL text,
where columns() asks SQLite.
@linusnorton

Copy link
Copy Markdown
Collaborator

issues.local.md

I've attached some issues, please have a look through. Let me know if you want me to take a look.

I'm a little concerned that trading the LOAD LOCAL DATA for INSERTS slows it down quite significantly but if that's the price of multi-db support it's probably worth it.

Four ways a flush could come back clean having written the wrong thing, none
of which the database reported.

A delete matches each row with its own bracketed group, ORed together. SQLite
parses that as a tree and refuses one deeper than 1000, which a two column key
reaches at 999 rows - inside a chunk the parameter limit allows, because 30000
bound values over the width of a row leaves 77 of the 80 declared tables with
chunks above it. The rows of such a statement are now capped as well as the
values it binds, and the chunks are sized off the values the statement actually
binds: a delete binds the key, not the row.

A key column that is null was matched with `column = ?`, which is unknown
rather than true on all three databases. The stored row is not found, so the
delete removes nothing, the insert behind it conflicts with nothing, and the
revision lands beside the row it was meant to replace. Seventeen of the
declared key columns are nullable, five of them in non_standard_discount
alone. They are matched with IS NULL now.

REPLACE took the last of the rows a statement gave it. Spelled out as a delete
and an insert it takes the first instead: the delete removes whatever was
stored, and the insert - which leaves any row already there alone - then
ignores every later revision of a key it has just written. A flush now keeps
the last revision of each key, which is what the statement it replaced did.

A flush is several statements whenever the rows bind more values than one may,
and the retry sends all of them again, so a lock error part way through a
flush re-inserted every chunk that had already committed. Eight declared
tables have no key, so nothing absorbed the duplicates. The whole flush is one
transaction now, which is the only thing that makes the retry safe.

The retry also never fired on SQLite: node:sqlite reports in errcode where
mysql2 and pg report in errno and code, so a locked file looked like any other
error. It is read now, along with MySQL's lock wait timeout, and the busy
timeout defaults to five seconds rather than node:sqlite's none at all, so
contention is something to wait out rather than fail on.
A file that raised was logged and stepped over: processFile caught the error,
printed it and returned, and the import then wrote the archive to the log
table and exited 0. The log table is what the next download starts from, so
recording an archive that did not import steps the cursor over every changes
file published between it and the next one. Nothing comes back for the rows
that were missed, and nothing says so.

Every file is still waited on before anything is raised, so one failure does
not hide the others and the errors are printed as before. What follows them
does not run: the archive is not recorded, the process exits non zero, and the
next run takes the same archive again.
The declaration language could not express seven things the DDL it replaced
said, so each of them was quietly dropped on the way through.

The engine and the character set. Every table used to end Engine=InnoDB, and
eleven of them DEFAULT CHARSET=utf8mb4. Without the first, a server whose
default engine is MyISAM or Aria gives the writer no transactions, and a
replace that fails after its delete leaves the rows gone; without the second,
a station name on a latin1 server is stored mangled. Both are MySQL's to
worry about, so the dialect says them and the other two say nothing.

The collation of an identifier. The ids the build composes were declared
CHARACTER SET ascii COLLATE ascii_bin, which the default collation is not:
under utf8mb4_general_ci, G38968_20261018 and g38968_20261018 are the same
value, so a primary key holding both rejects the second and a join on one
matches the other. Measured on 500k stop times, the default collation is also
1.6x slower to join on and 2.5x slower to count distinct. ascii() says an
identifier rather than prose, and only MySQL has to act on it: SQLite compares
byte for byte already and Postgres compares varchar exactly.

A default. transfers.from_trip_id and to_trip_id were NOT NULL DEFAULT '',
and the empty string is what keeps them in the primary key on a row that
describes a station interchange rather than a coupling. Declared without one
they became NOT NULL with no default, so a transfers.txt written without those
columns aborts the import.

A table with no surrogate key. network_flow_restriction had five columns
before it was declared here, and the builder adds the generated id to
everything, so it gained a sixth - first, shifting every field of a SELECT *
for anything reading it positionally. A declaration can now say it has none,
and the row type follows.

An exact number. SQLite was given decimal columns as numeric, which is the
affinity that converts what it is handed to a real - the one thing an exact
type exists not to do. A coordinate written as 57.480102 came back as the
number 57.480102 where the declaration says string, and where the other two
databases return the digits that were written. Stored as text, it is those
digits again.

A declared width. SQLite was also given every text column as a bare text,
so nothing reading the schema back could see what the column was for. It
still enforces nothing - that is SQLite - but it now says varchar(32) or
char(4) like the others.

A ceiling wide enough for what it holds. service_id is a counter over the
distinct calendars of a build rather than a four digit number, and integer(4)
is smallint to Postgres, which stops at 32767 where MySQL's unsigned one
reaches 65535. A national feed is close enough to both that the import fails
on one database and not the other, so it is declared as what it is.

The consistency test also compares variableLength now. It checked that a
column was text and wide enough but not that char and varchar agreed, and
they are not interchangeable: MySQL strips the trailing blanks of a char on
read, and the fixed width fields are read two characters at a time by the
callers that use them.
A GTFS time counts from the start of the service day rather than the clock,
so a sleeper calls at 24:01:00 and a late train at 26:15:00. MySQL's TIME
holds both, because it runs to 838 hours and was never a time of day; Postgres
refuses anything past 24:00:00. --gtfs-import therefore died part way through
stop_times on Postgres - 430 of the 1326 calls in the fixture feed are after
midnight - having already dropped and recreated the table. The columns are
text now, zero padded to eight characters, so ordering them as text is
ordering them by time.

Nothing caught it because nothing ran that import. check-database drives the
feed importer and the GTFS build, and --gtfs-import creates its own tables
from their own declarations, so the only thing exercising them was a unit test
against SQLite - which stores everything as text and would have accepted a
column type no server does. The script loads the feed it has just written back
in, which is the shortest path to running that DDL on all three.
An import writes rows that may already be there, so the insert says to leave
whatever is there alone. Two of the three spellings of that say a great deal
more.

MySQL's INSERT IGNORE downgrades every data error to a warning: a value too
long is stored truncated, one out of range is stored clamped, a null in a not
null column is stored as the empty string, and the import reports success.
SQLite's OR IGNORE skips such a row entirely and says nothing. Only Postgres'
ON CONFLICT DO NOTHING means what it says, and it is the behaviour the other
two are now asked for - SQLite by the same spelling, MySQL by ON DUPLICATE KEY
UPDATE setting a column to itself, which absorbs the duplicate key and raises
everything else.

That only has teeth where the database knows the width, and SQLite does not:
it stores a value of any length in a column of any width, so a row MySQL and
Postgres refuse was stored there in full and the three disagreed about what
had been imported. The declared width is now a constraint, which is the one
thing it does enforce.
Two ways the reader took a file it should have refused.

The specification permits a byte order mark, and nothing strips it, so the
first column of the header was named with one on the front. No table has that
column, so the import raised - but only after createSchema had dropped and
recreated the table, leaving it empty as well as unloaded.

A row with fewer values than the header was padded out with empty strings,
which a nullable column stored as null and a not null one stored as blank; a
row with more had the extra values dropped, because the reader walked the
header rather than the row. Neither said anything. A row that does not have
the header's columns is a file that is not what it says it is, and it is worth
stopping on while the rest of the feed is still intact.
The conversion named int, boolean, double, float, foreignKey and date, and
everything else fell through to the text default - which returns the empty
string where the column is not nullable. decimal and time are everything
else, and shapes.shape_pt_lat and shape_pt_lon are decimal and not null.

A coordinate the feed did not write was therefore written as "": MySQL stored
a zero, Postgres refused on the syntax, SQLite stored the empty string. Three
answers, none of them the truth, for a value nobody supplied.

Only a text column can hold the empty string, and only where it is declared
not to be nullable. Everywhere else an empty cell is a null, and a column that
cannot hold one now says so instead of storing a number the feed never gave.
The bound values are spread into node:sqlite as arguments, and it reads an
object in the first of them as a set of named parameters. A Date bound to the
first placeholder is therefore taken as no named parameters at all, every
later value slides one place towards it, and the row is stored with its
columns shifted and the last one null - with changes: 1 reported and no error
anywhere.

The same value in any other position throws. Anything that is not a string, a
number, a bigint, a buffer, a boolean or nothing at all is now refused here,
so the first position behaves like the rest of them.
getTransfers ranked the TIPLOCs of a CRS, took the winner, and only then
asked for one with a CATE interchange rating. Where the winner had none the
station dropped out of transfers.txt entirely, although the TIPLOC beside it
had a rating all along - and the ranking does not prefer a rated row, it
prefers a 9 and then the lowest TIPLOC code, so an unrated one wins often
enough.

Both of the older sources filter before they group, which is what this does
now: the rating is a condition on which rows are ranked rather than a test
applied to the row that won.
A field's null values are built at its full width, so a line that stops short
of it pads with a character that matches none of them: two spaces in a three
character field were a value rather than nothing. Since a text field drops
its padding, that value then parsed to the empty string - which a field
declared not nullable exists to refuse, and which changes the unique
constraint where the column is part of a key.

Nothing is now a run of one of the field's null characters, of any length. A
field declared with no null characters is untouched by that, which is the
distinction that matters: a blank base_location_suffix is a suffix rather
than a missing value, and it still arrives as the empty string.

ZeroFillIntField also extends TextField rather than Field, so it drops its
padding like every other value stored as text. It is zero filled on the way
out either way, and it was the one text column keeping the blanks the rest
had stopped keeping.
The dates a row works out to are a function of three of its columns, so the
rows sharing those three all work out to the same pair. The command read every
row, grouped their ids by the resulting pair and then updated by id - and the
grouping accumulated each group by copying the array it had so far, which is
quadratic in the rows of a group.

A feed has a hundred thousand rows and four combinations, so that is five
billion element copies to learn four things the database could have been asked
for. Measured against MariaDB on 100,000 rows: 9.00s to group and update by
id, 0.26s to select the distinct combinations and update by them.

The ids are never read, held or sent back now, which also takes away the limit
on how many of them one statement may name and the loop that sliced them up.
The table is dropped and recreated a moment before the file is loaded into
it, and the load is one statement per five thousand rows, so a failure part
way through left the table holding some of the file with nothing to say which
part. It is the file or it is the empty table it was.

Being one transaction rather than a hundred is also slightly quicker - 500,000
rows in 9.6s rather than 10.3s against MariaDB - but the atomicity is the
point. LOAD DATA LOCAL INFILE reads the same file in 3.7s, which is the price
of the import being one code path that three databases run rather than a
MySQL client invocation.
Four ways the environment was read differently from how it is documented.

DATABASE_OPTIONS did not win. Handed a uri, mysql2 merges the parsed URL over
the options it was given on truthiness - connection_config.js is `if
(options[key]) continue` - so a false or an empty string lost to the URL,
where this file and the README both say the options are the escape hatch that
overrides. The URL is read with mysql2's own parser here and spread under the
options, so every URL it accepts still works and the order is the order.

The URL's scheme was only read when DATABASE_DIALECT was unset, so setting the
dialect skipped the check that exists to stop a Postgres URL reaching mysql2 -
the precise failure the comment above it describes. It is read either way now,
and a dialect contradicting the scheme is an error rather than a silent choice
between them.

A SQLite URL carrying parameters became a filename with a question mark in it:
file:feed.db?mode=ro opened a file called feed.db?mode=ro. node:sqlite takes a
path rather than a URI, so that is refused and says where to put the option
instead.

multipleStatements is gone. Nothing sends more than one statement per query
any more - the DDL goes through Kysely one at a time and the two raw queries
left are single selects - and it widens what a future injection could reach.

databaseConfigured() is here rather than at its call site for the same reason
as the rest of it: this is what reads the environment, and asking it about one
of the three ways in is what the next commit fixes.
Three things the container got wrong about the connections it opens.

The feed cursor was taken from DATABASE_NAME alone. A DATABASE_URL install is
fully configured and had no cursor, so every download took the most recent
full refresh again and skipped every changes file published between them -
quietly, because that is also what a first run looks like. It asks whether a
database is configured now, by either of the ways of configuring one.

The SQLite database was opened as the dialect was built, and never closed.
Kysely's SQLite dialect takes a function returning the database and calls it
when the first query runs, which is what it is given now - so a command that
runs no query, and --download-timetable is one, no longer leaves an empty
database file behind. Closing it needed the tracker to accept something with
destroy() rather than end(): a pool and a file handle are not the same thing,
and the Kysely is what holds the file.

pg-cursor was optional all the way to the point of use. Kysely's Postgres
dialect streams only when it has one, and the GTFS build streams the stop
times of a whole feed, so a missing package failed part way through a build
with a message naming neither the package nor the fix. The build asks for it
up front, where the error says which package to install.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants