From ba9822899a1045b51f213156eb9a58ffa212623c Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 12 Oct 2022 12:31:43 +0100 Subject: [PATCH] Fix pagination --- syncapi/storage/postgres/relations_table.go | 49 +++++++++++++++----- syncapi/storage/shared/storage_sync.go | 23 ++++++---- syncapi/storage/sqlite3/relations_table.go | 51 ++++++++++++++++----- 3 files changed, 90 insertions(+), 33 deletions(-) diff --git a/syncapi/storage/postgres/relations_table.go b/syncapi/storage/postgres/relations_table.go index 934fddbf0..830158921 100644 --- a/syncapi/storage/postgres/relations_table.go +++ b/syncapi/storage/postgres/relations_table.go @@ -48,25 +48,37 @@ const insertRelationSQL = "" + const deleteRelationSQL = "" + "DELETE FROM syncapi_relations WHERE room_id = $1 AND child_event_id = $2" -const selectRelationsInRangeSQL = "" + +const selectRelationsInRangeAscSQL = "" + "SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" + " WHERE room_id = $1 AND event_id = $2 AND id > $3 AND id <= $4" + + " ORDER BY id ASC LIMIT $5" + +const selectRelationsInRangeDescSQL = "" + + "SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" + + " WHERE room_id = $1 AND event_id = $2 AND id >= $3 AND id < $4" + " ORDER BY id DESC LIMIT $5" -const selectRelationsByTypeInRangeSQL = "" + +const selectRelationsByTypeInRangeAscSQL = "" + "SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" + " WHERE room_id = $1 AND event_id = $2 AND rel_type = $3 AND id > $4 AND id <= $5" + + " ORDER BY id ASC LIMIT $6" + +const selectRelationsByTypeInRangeDescSQL = "" + + "SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" + + " WHERE room_id = $1 AND event_id = $2 AND rel_type = $3 AND id >= $4 AND id < $5" + " ORDER BY id DESC LIMIT $6" const selectMaxRelationIDSQL = "" + "SELECT MAX(id) FROM syncapi_relations" type relationsStatements struct { - insertRelationStmt *sql.Stmt - selectRelationsInRangeStmt *sql.Stmt - selectRelationsByTypeInRangeStmt *sql.Stmt - deleteRelationStmt *sql.Stmt - selectMaxRelationIDStmt *sql.Stmt + insertRelationStmt *sql.Stmt + selectRelationsInRangeAscStmt *sql.Stmt + selectRelationsInRangeDescStmt *sql.Stmt + selectRelationsByTypeInRangeAscStmt *sql.Stmt + selectRelationsByTypeInRangeDescStmt *sql.Stmt + deleteRelationStmt *sql.Stmt + selectMaxRelationIDStmt *sql.Stmt } func NewPostgresRelationsTable(db *sql.DB) (tables.Relations, error) { @@ -78,10 +90,16 @@ func NewPostgresRelationsTable(db *sql.DB) (tables.Relations, error) { if s.insertRelationStmt, err = db.Prepare(insertRelationSQL); err != nil { return nil, err } - if s.selectRelationsInRangeStmt, err = db.Prepare(selectRelationsInRangeSQL); err != nil { + if s.selectRelationsInRangeAscStmt, err = db.Prepare(selectRelationsInRangeAscSQL); err != nil { return nil, err } - if s.selectRelationsByTypeInRangeStmt, err = db.Prepare(selectRelationsByTypeInRangeSQL); err != nil { + if s.selectRelationsInRangeDescStmt, err = db.Prepare(selectRelationsInRangeDescSQL); err != nil { + return nil, err + } + if s.selectRelationsByTypeInRangeAscStmt, err = db.Prepare(selectRelationsByTypeInRangeAscSQL); err != nil { + return nil, err + } + if s.selectRelationsByTypeInRangeDescStmt, err = db.Prepare(selectRelationsByTypeInRangeDescSQL); err != nil { return nil, err } if s.deleteRelationStmt, err = db.Prepare(deleteRelationSQL); err != nil { @@ -121,13 +139,22 @@ func (s *relationsStatements) SelectRelationsInRange( r types.Range, limit int, ) (map[string][]types.RelationEntry, types.StreamPosition, error) { var lastPos types.StreamPosition + var stmt *sql.Stmt var rows *sql.Rows var err error if relType != "" { - stmt := sqlutil.TxStmt(txn, s.selectRelationsByTypeInRangeStmt) + if r.Backwards { + stmt = sqlutil.TxStmt(txn, s.selectRelationsByTypeInRangeDescStmt) + } else { + stmt = sqlutil.TxStmt(txn, s.selectRelationsByTypeInRangeAscStmt) + } rows, err = stmt.QueryContext(ctx, roomID, eventID, relType, r.Low(), r.High(), limit) } else { - stmt := sqlutil.TxStmt(txn, s.selectRelationsInRangeStmt) + if r.Backwards { + stmt = sqlutil.TxStmt(txn, s.selectRelationsInRangeDescStmt) + } else { + stmt = sqlutil.TxStmt(txn, s.selectRelationsInRangeAscStmt) + } rows, err = stmt.QueryContext(ctx, roomID, eventID, r.Low(), r.High(), limit) } if err != nil { diff --git a/syncapi/storage/shared/storage_sync.go b/syncapi/storage/shared/storage_sync.go index a42ceb79d..4a9d7143d 100644 --- a/syncapi/storage/shared/storage_sync.go +++ b/syncapi/storage/shared/storage_sync.go @@ -605,13 +605,18 @@ func (d *DatabaseTransaction) RelationsFor(ctx context.Context, roomID, eventID, To: to, Backwards: backwards, } - if r.To == 0 && !backwards { - if r.To, err = d.MaxStreamPositionForRelations(ctx); err != nil { - return nil, "", "", fmt.Errorf("d.MaxStreamPositionForRelations: %w", err) + if r.Backwards { + if r.From == 0 { + if r.From, err = d.MaxStreamPositionForRelations(ctx); err != nil { + return nil, "", "", fmt.Errorf("d.MaxStreamPositionForRelations: %w", err) + } + r.From++ } - } else if r.From == 0 && backwards { - if r.From, err = d.MaxStreamPositionForRelations(ctx); err != nil { - return nil, "", "", fmt.Errorf("d.MaxStreamPositionForRelations: %w", err) + } else { + if r.To == 0 { + if r.To, err = d.MaxStreamPositionForRelations(ctx); err != nil { + return nil, "", "", fmt.Errorf("d.MaxStreamPositionForRelations: %w", err) + } } } @@ -641,13 +646,11 @@ func (d *DatabaseTransaction) RelationsFor(ctx context.Context, roomID, eventID, // Otherwise, let's try and work out what sensible prev_batch and next_batch values // could be. - if from > 0 { - prevBatch = fmt.Sprintf("%d", r.Low()-1) - } if len(entries) > limit { - nextBatch = fmt.Sprintf("%d", entries[len(entries)-1].Position) entries = entries[:len(entries)-1] + nextBatch = fmt.Sprintf("%d", entries[len(entries)-1].Position) } + // TODO: set prevBatch? doesn't seem to affect the tests... // Extract all of the event IDs from the relation entries so that we can pull the // events out of the database. diff --git a/syncapi/storage/sqlite3/relations_table.go b/syncapi/storage/sqlite3/relations_table.go index 4f25777ea..349e5171a 100644 --- a/syncapi/storage/sqlite3/relations_table.go +++ b/syncapi/storage/sqlite3/relations_table.go @@ -46,26 +46,38 @@ const insertRelationSQL = "" + const deleteRelationSQL = "" + "DELETE FROM syncapi_relations WHERE room_id = $1 AND child_event_id = $2" -const selectRelationsInRangeSQL = "" + +const selectRelationsInRangeAscSQL = "" + "SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" + " WHERE room_id = $1 AND event_id = $2 AND id > $3 AND id <= $4" + + " ORDER BY id ASC LIMIT $5" + +const selectRelationsInRangeDescSQL = "" + + "SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" + + " WHERE room_id = $1 AND event_id = $2 AND id >= $3 AND id < $4" + " ORDER BY id DESC LIMIT $5" -const selectRelationsByTypeInRangeSQL = "" + +const selectRelationsByTypeInRangeAscSQL = "" + "SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" + " WHERE room_id = $1 AND event_id = $2 AND rel_type = $3 AND id > $4 AND id <= $5" + + " ORDER BY id ASC LIMIT $6" + +const selectRelationsByTypeInRangeDescSQL = "" + + "SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" + + " WHERE room_id = $1 AND event_id = $2 AND rel_type = $3 AND id >= $4 AND id < $5" + " ORDER BY id DESC LIMIT $6" const selectMaxRelationIDSQL = "" + "SELECT MAX(id) FROM syncapi_relations" type relationsStatements struct { - streamIDStatements *StreamIDStatements - insertRelationStmt *sql.Stmt - selectRelationsInRangeStmt *sql.Stmt - selectRelationsByTypeInRangeStmt *sql.Stmt - deleteRelationStmt *sql.Stmt - selectMaxRelationIDStmt *sql.Stmt + streamIDStatements *StreamIDStatements + insertRelationStmt *sql.Stmt + selectRelationsInRangeAscStmt *sql.Stmt + selectRelationsInRangeDescStmt *sql.Stmt + selectRelationsByTypeInRangeAscStmt *sql.Stmt + selectRelationsByTypeInRangeDescStmt *sql.Stmt + deleteRelationStmt *sql.Stmt + selectMaxRelationIDStmt *sql.Stmt } func NewSqliteRelationsTable(db *sql.DB, streamID *StreamIDStatements) (tables.Relations, error) { @@ -79,10 +91,16 @@ func NewSqliteRelationsTable(db *sql.DB, streamID *StreamIDStatements) (tables.R if s.insertRelationStmt, err = db.Prepare(insertRelationSQL); err != nil { return nil, err } - if s.selectRelationsInRangeStmt, err = db.Prepare(selectRelationsInRangeSQL); err != nil { + if s.selectRelationsInRangeAscStmt, err = db.Prepare(selectRelationsInRangeAscSQL); err != nil { return nil, err } - if s.selectRelationsByTypeInRangeStmt, err = db.Prepare(selectRelationsByTypeInRangeSQL); err != nil { + if s.selectRelationsInRangeDescStmt, err = db.Prepare(selectRelationsInRangeDescSQL); err != nil { + return nil, err + } + if s.selectRelationsByTypeInRangeAscStmt, err = db.Prepare(selectRelationsByTypeInRangeAscSQL); err != nil { + return nil, err + } + if s.selectRelationsByTypeInRangeDescStmt, err = db.Prepare(selectRelationsByTypeInRangeDescSQL); err != nil { return nil, err } if s.deleteRelationStmt, err = db.Prepare(deleteRelationSQL); err != nil { @@ -125,13 +143,22 @@ func (s *relationsStatements) SelectRelationsInRange( r types.Range, limit int, ) (map[string][]types.RelationEntry, types.StreamPosition, error) { var lastPos types.StreamPosition + var stmt *sql.Stmt var rows *sql.Rows var err error if relType != "" { - stmt := sqlutil.TxStmt(txn, s.selectRelationsByTypeInRangeStmt) + if r.Backwards { + stmt = sqlutil.TxStmt(txn, s.selectRelationsByTypeInRangeDescStmt) + } else { + stmt = sqlutil.TxStmt(txn, s.selectRelationsByTypeInRangeAscStmt) + } rows, err = stmt.QueryContext(ctx, roomID, eventID, relType, r.Low(), r.High(), limit) } else { - stmt := sqlutil.TxStmt(txn, s.selectRelationsInRangeStmt) + if r.Backwards { + stmt = sqlutil.TxStmt(txn, s.selectRelationsInRangeDescStmt) + } else { + stmt = sqlutil.TxStmt(txn, s.selectRelationsInRangeAscStmt) + } rows, err = stmt.QueryContext(ctx, roomID, eventID, r.Low(), r.High(), limit) } if err != nil {