From 26688ffe65a825f79199fcd593aac06a4d14711a Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Tue, 20 Oct 2026 18:37:24 -0700 Subject: [PATCH 1/4] Index block_trip_entry by block_id GetBlocksForBlockTripIndexIDs, which trips-for-route uses to find a route's active blocks, joins block_trip_entry to itself on block_id. With no index there SQLite scanned every block entry for each route trip: on NJ Transit's bus feed (33k trips) trips-for-route took 0.65 s locally and 4-5 s in production for five trips. With the index it takes 0.05 s. schema.sql runs at startup, so existing databases get the index too. --- gtfsdb/block_trip_entry_index_test.go | 38 +++++++++++++++++++++++++++ gtfsdb/schema.sql | 5 ++++ 2 files changed, 43 insertions(+) create mode 100644 gtfsdb/block_trip_entry_index_test.go diff --git a/gtfsdb/block_trip_entry_index_test.go b/gtfsdb/block_trip_entry_index_test.go new file mode 100644 index 000000000..6bc2d862d --- /dev/null +++ b/gtfsdb/block_trip_entry_index_test.go @@ -0,0 +1,38 @@ +package gtfsdb + +import ( + "context" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "maglev.onebusaway.org/internal/appconf" +) + +// GetBlocksForBlockTripIndexIDs joins block_trip_entry to itself on block_id. Without +// an index there, SQLite scans every block entry for every route trip, which made +// trips-for-route take seconds on NJ Transit's 33k-trip feed. +func TestBlockTripEntryBlockIDJoinUsesIndex(t *testing.T) { + client, err := NewClient(Config{DBPath: ":memory:", Env: appconf.Test}) + require.NoError(t, err) + t.Cleanup(func() { _ = client.Close() }) + + rows, err := client.DB.QueryContext(context.Background(), `EXPLAIN QUERY PLAN + SELECT DISTINCT a.block_id FROM block_trip_entry a + JOIN block_trip_entry b ON b.block_id = a.block_id + WHERE a.trip_id = 't1'`) + require.NoError(t, err) + defer func() { _ = rows.Close() }() + + var plan []string + for rows.Next() { + var id, parent, notUsed int + var detail string + require.NoError(t, rows.Scan(&id, &parent, ¬Used, &detail)) + plan = append(plan, detail) + } + require.NoError(t, rows.Err()) + + assert.Contains(t, strings.Join(plan, "\n"), "idx_block_trip_entry_block_id") +} diff --git a/gtfsdb/schema.sql b/gtfsdb/schema.sql index d55ca2bed..12d1a42b7 100644 --- a/gtfsdb/schema.sql +++ b/gtfsdb/schema.sql @@ -454,6 +454,11 @@ CREATE INDEX IF NOT EXISTS idx_block_trip_index_service_ids ON block_trip_index -- migrate CREATE INDEX IF NOT EXISTS idx_block_trip_entry_index_id ON block_trip_entry (block_trip_index_id); +-- migrate +-- Block-wide joins (e.g. GetBlocksForBlockTripIndexIDs, which finds a route's +-- active blocks for trips-for-route) match block_trip_entry rows by block_id. +CREATE INDEX IF NOT EXISTS idx_block_trip_entry_block_id ON block_trip_entry (block_id); + -- migrate CREATE INDEX IF NOT EXISTS idx_block_trip_entry_trip_id ON block_trip_entry (trip_id); From 68abc72d309b16704a529a69ac67c5c1f2b0e53a Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Sat, 7 Nov 2026 10:27:50 -0800 Subject: [PATCH 2/4] Explain the real query in the block_id index test The test explained a simplified self-join that omitted the trip joins and filters of GetBlocksForBlockTripIndexIDs, so it could pass on a plan the real query never uses. Explain the sqlc-generated query instead, with one value in each IN list, and require SQLite to search route_bte by block_id through idx_block_trip_entry_block_id. --- gtfsdb/block_trip_entry_index_test.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/gtfsdb/block_trip_entry_index_test.go b/gtfsdb/block_trip_entry_index_test.go index 6bc2d862d..3a8ee6d6f 100644 --- a/gtfsdb/block_trip_entry_index_test.go +++ b/gtfsdb/block_trip_entry_index_test.go @@ -2,6 +2,7 @@ package gtfsdb import ( "context" + "regexp" "strings" "testing" @@ -18,10 +19,14 @@ func TestBlockTripEntryBlockIDJoinUsesIndex(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { _ = client.Close() }) - rows, err := client.DB.QueryContext(context.Background(), `EXPLAIN QUERY PLAN - SELECT DISTINCT a.block_id FROM block_trip_entry a - JOIN block_trip_entry b ON b.block_id = a.block_id - WHERE a.trip_id = 't1'`) + // Explain the generated query itself, with one value in each IN list, so the + // test follows any change to its joins and filters. + sliceMarker := regexp.MustCompile(`/\*SLICE:\w+\*/\?`) + query := sliceMarker.ReplaceAllString(getBlocksForBlockTripIndexIDs, "?") + fromTime, toTime, routeID := int64(0), int64(3600), "route_1" + indexID, activeServiceID, routeServiceID := int64(1), "service_1", "service_1" + rows, err := client.DB.QueryContext(context.Background(), "EXPLAIN QUERY PLAN "+query, + fromTime, toTime, routeID, indexID, activeServiceID, routeServiceID) require.NoError(t, err) defer func() { _ = rows.Close() }() @@ -34,5 +39,6 @@ func TestBlockTripEntryBlockIDJoinUsesIndex(t *testing.T) { } require.NoError(t, rows.Err()) - assert.Contains(t, strings.Join(plan, "\n"), "idx_block_trip_entry_block_id") + assert.Contains(t, strings.Join(plan, "\n"), + "SEARCH route_bte USING INDEX idx_block_trip_entry_block_id (block_id=?)") } From 83338217e3ed42dec55e632197dc4b8f0a74d0f5 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Sat, 7 Nov 2026 13:09:57 -0800 Subject: [PATCH 3/4] Check the block_id index plan on the RABA feed The plan test explained the query on an empty database, rebuilt the SQL by rewriting sqlc's slice markers with a regex, and required the index on the route_bte alias. On real data, and after ANALYZE, SQLite can start the self-join from route_bte and put the index on active_bte instead, which failed the test although the index was in use. Load the RABA feed, call the generated GetBlocksForBlockTripIndexIDs method through a recorder to capture its exact SQL and arguments, and accept a block_id index search on either alias. The recorder and the EXPLAIN QUERY PLAN scan move to query_plan_test.go so the rtree plan test shares them. --- gtfsdb/block_trip_entry_index_test.go | 50 +++++++++++++-------------- gtfsdb/query_plan_test.go | 41 ++++++++++++++++++++++ gtfsdb/stops_rtree_test.go | 23 ++---------- 3 files changed, 67 insertions(+), 47 deletions(-) create mode 100644 gtfsdb/query_plan_test.go diff --git a/gtfsdb/block_trip_entry_index_test.go b/gtfsdb/block_trip_entry_index_test.go index 3a8ee6d6f..c3e5a7e0b 100644 --- a/gtfsdb/block_trip_entry_index_test.go +++ b/gtfsdb/block_trip_entry_index_test.go @@ -1,44 +1,42 @@ package gtfsdb import ( - "context" - "regexp" + "database/sql" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "maglev.onebusaway.org/internal/appconf" ) // GetBlocksForBlockTripIndexIDs joins block_trip_entry to itself on block_id. Without // an index there, SQLite scans every block entry for every route trip, which made // trips-for-route take seconds on NJ Transit's 33k-trip feed. func TestBlockTripEntryBlockIDJoinUsesIndex(t *testing.T) { - client, err := NewClient(Config{DBPath: ":memory:", Env: appconf.Test}) - require.NoError(t, err) - t.Cleanup(func() { _ = client.Close() }) + client := newTestClientWithRABA(t) + ctx := t.Context() - // Explain the generated query itself, with one value in each IN list, so the - // test follows any change to its joins and filters. - sliceMarker := regexp.MustCompile(`/\*SLICE:\w+\*/\?`) - query := sliceMarker.ReplaceAllString(getBlocksForBlockTripIndexIDs, "?") - fromTime, toTime, routeID := int64(0), int64(3600), "route_1" - indexID, activeServiceID, routeServiceID := int64(1), "service_1", "service_1" - rows, err := client.DB.QueryContext(context.Background(), "EXPLAIN QUERY PLAN "+query, - fromTime, toTime, routeID, indexID, activeServiceID, routeServiceID) - require.NoError(t, err) - defer func() { _ = rows.Close() }() + var routeID, serviceID string + var indexID int64 + require.NoError(t, client.DB.QueryRowContext(ctx, ` + SELECT trips.route_id, bte.service_id, bte.block_trip_index_id + FROM block_trip_entry bte JOIN trips ON trips.id = bte.trip_id + WHERE bte.block_id IS NOT NULL LIMIT 1`).Scan(&routeID, &serviceID, &indexID)) - var plan []string - for rows.Next() { - var id, parent, notUsed int - var detail string - require.NoError(t, rows.Scan(&id, &parent, ¬Used, &detail)) - plan = append(plan, detail) - } - require.NoError(t, rows.Err()) + recorder := &queryRecorder{DBTX: client.DB} + _, err := New(recorder).GetBlocksForBlockTripIndexIDs(ctx, GetBlocksForBlockTripIndexIDsParams{ + FromTime: sql.NullInt64{Int64: 0, Valid: true}, + ToTime: sql.NullInt64{Int64: int64(24 * time.Hour), Valid: true}, + RouteID: routeID, + IndexIds: []int64{indexID}, + ActiveServiceIds: []string{serviceID}, + RouteServiceIds: []string{serviceID}, + }) + require.NoError(t, err) - assert.Contains(t, strings.Join(plan, "\n"), - "SEARCH route_bte USING INDEX idx_block_trip_entry_block_id (block_id=?)") + // The planner may start the self-join from either side, so accept the index + // on either alias. + plan := strings.Join(explainRecordedQuery(t, client, recorder), "\n") + assert.Regexp(t, `SEARCH \w+ USING (COVERING )?INDEX idx_block_trip_entry_block_id \(block_id=\?`, plan) } diff --git a/gtfsdb/query_plan_test.go b/gtfsdb/query_plan_test.go new file mode 100644 index 000000000..9e25181ef --- /dev/null +++ b/gtfsdb/query_plan_test.go @@ -0,0 +1,41 @@ +package gtfsdb + +import ( + "context" + "database/sql" + "testing" + + "github.com/stretchr/testify/require" +) + +// queryRecorder captures the SQL and arguments a generated query method sends, +// so a test can explain the real query rather than a copy of it. +type queryRecorder struct { + DBTX + query string + args []any +} + +func (db *queryRecorder) QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error) { + db.query, db.args = query, args + return db.DBTX.QueryContext(ctx, query, args...) +} + +// explainRecordedQuery returns the detail column of each EXPLAIN QUERY PLAN row +// for the query the recorder last captured. +func explainRecordedQuery(t *testing.T, client *Client, recorder *queryRecorder) []string { + t.Helper() + rows, err := client.DB.QueryContext(t.Context(), "EXPLAIN QUERY PLAN "+recorder.query, recorder.args...) + require.NoError(t, err) + defer func() { _ = rows.Close() }() + + var details []string + for rows.Next() { + var id, parent, unused int + var detail string + require.NoError(t, rows.Scan(&id, &parent, &unused, &detail)) + details = append(details, detail) + } + require.NoError(t, rows.Err()) + return details +} diff --git a/gtfsdb/stops_rtree_test.go b/gtfsdb/stops_rtree_test.go index a82fdc64a..6bd190e89 100644 --- a/gtfsdb/stops_rtree_test.go +++ b/gtfsdb/stops_rtree_test.go @@ -2,7 +2,6 @@ package gtfsdb import ( "context" - "database/sql" "strings" "testing" @@ -47,44 +46,26 @@ func TestGetActiveRoutesWithinBounds_CenteredOnStop(t *testing.T) { assert.ElementsMatch(t, []string{"15", "154", "159", "160", "44X"}, stop4000RouteIDs) } -type routeQueryRecorder struct { - DBTX - query string - args []any -} - -func (db *routeQueryRecorder) QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error) { - db.query, db.args = query, args - return db.DBTX.QueryContext(ctx, query, args...) -} - func TestGetActiveRoutesWithinBounds_FilteredStopTimesLookup(t *testing.T) { client := newTestClientWithRABA(t) ctx := t.Context() var routeID string require.NoError(t, client.DB.QueryRowContext(ctx, "SELECT id FROM routes LIMIT 1").Scan(&routeID)) - recorder := &routeQueryRecorder{DBTX: client.DB} + recorder := &queryRecorder{DBTX: client.DB} _, err := New(recorder).GetActiveRoutesWithinBounds(ctx, GetActiveRoutesWithinBoundsParams{ Lat: 40.7347, Lon: -74.1644, MinLat: -90, MaxLat: 90, MinLon: -180, MaxLon: 180, RouteIDs: []string{routeID}, MaxCount: 100, }) require.NoError(t, err) - plan, err := client.DB.QueryContext(ctx, "EXPLAIN QUERY PLAN "+recorder.query, recorder.args...) - require.NoError(t, err) - defer func() { _ = plan.Close() }() var stopTimeLookups []string - for plan.Next() { - var id, parent, unused int - var detail string - require.NoError(t, plan.Scan(&id, &parent, &unused, &detail)) + for _, detail := range explainRecordedQuery(t, client, recorder) { if strings.Contains(detail, "stop_times") { stopTimeLookups = append(stopTimeLookups, detail) } } - require.NoError(t, plan.Err()) require.NotEmpty(t, stopTimeLookups) for _, lookup := range stopTimeLookups { assert.Contains(t, lookup, "(trip_id=?)", "filtered searches must look up stop times by matching trip, not nearby stop") From 5c1e5aeb80eccd6678b8a5b9051810f6b6094902 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Sat, 7 Nov 2026 13:09:57 -0800 Subject: [PATCH 4/4] Name both block_id queries in the index comment GetBlockTripIndexIDsForBlocks also matches block_trip_entry rows by block_id, so list it next to GetBlocksForBlockTripIndexIDs above idx_block_trip_entry_block_id. --- gtfsdb/schema.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gtfsdb/schema.sql b/gtfsdb/schema.sql index 12d1a42b7..39ea32156 100644 --- a/gtfsdb/schema.sql +++ b/gtfsdb/schema.sql @@ -455,8 +455,8 @@ CREATE INDEX IF NOT EXISTS idx_block_trip_index_service_ids ON block_trip_index CREATE INDEX IF NOT EXISTS idx_block_trip_entry_index_id ON block_trip_entry (block_trip_index_id); -- migrate --- Block-wide joins (e.g. GetBlocksForBlockTripIndexIDs, which finds a route's --- active blocks for trips-for-route) match block_trip_entry rows by block_id. +-- GetBlocksForBlockTripIndexIDs (a route's active blocks, for trips-for-route) +-- and GetBlockTripIndexIDsForBlocks both match block_trip_entry rows by block_id. CREATE INDEX IF NOT EXISTS idx_block_trip_entry_block_id ON block_trip_entry (block_id); -- migrate