diff --git a/CHANGELOG.md b/CHANGELOG.md index e7ed2b41..8cb44de4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,21 @@ - Added support for wrapped API errors (_in case Go 1.20+ is used with multiple wrapped errors, `apis.ApiError` takes precedence_). +- Changes to the List/Search APIs + + - Increased the max allowed `?perPage` limit to 1000. + + - Reverted the default `COUNT` column to `id` as there are some common situations where it can negatively impact the query performance. + Additionally, from this version we also set `PRAGMA temp_store = MEMORY` so that also helps with the temp B-TREE creation when `id` is used. + _There are still scenarios where `COUNT` queries with `rowid` executes faster, but the majority of the time when nested relations lookups are used it seems to have the opposite effect (at least based on the benchmarks dataset)._ + + - The count and regular select statements also now executes concurrently, meaning that we no longer perform normalization over the `page` parameter and in case the user + request a page that doesn't exist (eg. `?page=99999999`) we'll return empty `items` array. + + - (@todo docs) Added new query parameter `?skipTotal=1` to skip the `COUNT` query performed with the list/search actions ([#2965](https://github.com/pocketbase/pocketbase/discussions/2965)). + If `?skipTotal=1` is set, the response fields `totalItems` and `totalPages` will have `-1` value (this is to avoid having different JSON responses and to differentiate from the zero default). + With the latest JS SDK 0.16+ and Dart SDK v0.11+ versions `skipTotal=1` is set by default for the `getFirstListItem()` and `getFullList()` requests. + ## v0.16.10 diff --git a/apis/record_crud.go b/apis/record_crud.go index d35fef8b..991e5abe 100644 --- a/apis/record_crud.go +++ b/apis/record_crud.go @@ -68,11 +68,6 @@ func (api *recordApi) list(c echo.Context) error { searchProvider := search.NewProvider(fieldsResolver). Query(api.app.Dao().RecordQuery(collection)) - // views don't have "rowid" so we fallback to "id" - if collection.IsView() { - searchProvider.CountCol("id") - } - if requestInfo.Admin == nil && collection.ListRule != nil { searchProvider.AddFilter(search.FilterData(*collection.ListRule)) } diff --git a/core/db_cgo.go b/core/db_cgo.go index 7c4f7abc..4165ac24 100644 --- a/core/db_cgo.go +++ b/core/db_cgo.go @@ -28,6 +28,7 @@ func init() { PRAGMA journal_size_limit = 200000000; PRAGMA synchronous = NORMAL; PRAGMA foreign_keys = ON; + PRAGMA temp_store = MEMORY; `, nil) return err diff --git a/core/db_nocgo.go b/core/db_nocgo.go index 5ab2bb05..9135db3c 100644 --- a/core/db_nocgo.go +++ b/core/db_nocgo.go @@ -11,7 +11,7 @@ func connectDB(dbPath string) (*dbx.DB, error) { // Note: the busy_timeout pragma must be first because // the connection needs to be set to block on busy before WAL mode // is set in case it hasn't been already set by another connection. - pragmas := "?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL)&_pragma=journal_size_limit(200000000)&_pragma=synchronous(NORMAL)&_pragma=foreign_keys(ON)" + pragmas := "?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL)&_pragma=journal_size_limit(200000000)&_pragma=synchronous(NORMAL)&_pragma=foreign_keys(ON)&_pragma=temp_store(MEMORY)" db, err := dbx.Open("sqlite", dbPath+pragmas) if err != nil { diff --git a/tools/search/provider.go b/tools/search/provider.go index 9c6a482a..48523795 100644 --- a/tools/search/provider.go +++ b/tools/search/provider.go @@ -7,20 +7,22 @@ import ( "strconv" "github.com/pocketbase/dbx" + "golang.org/x/sync/errgroup" ) // DefaultPerPage specifies the default returned search result items. const DefaultPerPage int = 30 // MaxPerPage specifies the maximum allowed search result items returned in a single page. -const MaxPerPage int = 500 +const MaxPerPage int = 1000 // url search query params const ( - PageQueryParam string = "page" - PerPageQueryParam string = "perPage" - SortQueryParam string = "sort" - FilterQueryParam string = "filter" + PageQueryParam string = "page" + PerPageQueryParam string = "perPage" + SortQueryParam string = "sort" + FilterQueryParam string = "filter" + SkipTotalQueryParam string = "skipTotal" ) // Result defines the returned search result structure. @@ -36,6 +38,7 @@ type Result struct { type Provider struct { fieldResolver FieldResolver query *dbx.SelectQuery + skipTotal bool countCol string page int perPage int @@ -57,7 +60,7 @@ type Provider struct { func NewProvider(fieldResolver FieldResolver) *Provider { return &Provider{ fieldResolver: fieldResolver, - countCol: "_rowid_", + countCol: "id", page: 1, perPage: DefaultPerPage, sort: []SortField{}, @@ -71,8 +74,16 @@ func (s *Provider) Query(query *dbx.SelectQuery) *Provider { return s } -// CountCol allows changing the default column (_rowid_) that is used +// SkipTotal changes the `skipTotal` field of the current search provider. +func (s *Provider) SkipTotal(skipTotal bool) *Provider { + s.skipTotal = skipTotal + return s +} + +// CountCol allows changing the default column (id) that is used // to generated the COUNT SQL query statement. +// +// This field is ignored if skipTotal is true. func (s *Provider) CountCol(name string) *Provider { s.countCol = name return s @@ -132,30 +143,38 @@ func (s *Provider) Parse(urlQuery string) error { return err } - if rawPage := params.Get(PageQueryParam); rawPage != "" { - page, err := strconv.Atoi(rawPage) + if raw := params.Get(SkipTotalQueryParam); raw != "" { + v, err := strconv.ParseBool(raw) if err != nil { return err } - s.Page(page) + s.SkipTotal(v) } - if rawPerPage := params.Get(PerPageQueryParam); rawPerPage != "" { - perPage, err := strconv.Atoi(rawPerPage) + if raw := params.Get(PageQueryParam); raw != "" { + v, err := strconv.Atoi(raw) if err != nil { return err } - s.PerPage(perPage) + s.Page(v) } - if rawSort := params.Get(SortQueryParam); rawSort != "" { - for _, sortField := range ParseSortFromString(rawSort) { + if raw := params.Get(PerPageQueryParam); raw != "" { + v, err := strconv.Atoi(raw) + if err != nil { + return err + } + s.PerPage(v) + } + + if raw := params.Get(SortQueryParam); raw != "" { + for _, sortField := range ParseSortFromString(raw) { s.AddSort(sortField) } } - if rawFilter := params.Get(FilterQueryParam); rawFilter != "" { - s.AddFilter(FilterData(rawFilter)) + if raw := params.Get(FilterQueryParam); raw != "" { + s.AddFilter(FilterData(raw)) } return nil @@ -165,10 +184,10 @@ func (s *Provider) Parse(urlQuery string) error { // the provided `items` slice with the found models. func (s *Provider) Exec(items any) (*Result, error) { if s.query == nil { - return nil, errors.New("Query is not set.") + return nil, errors.New("query is not set") } - // clone provider's query + // shallow clone the provider's query modelsQuery := *s.query // build filters @@ -198,18 +217,9 @@ func (s *Provider) Exec(items any) (*Result, error) { return nil, err } - queryInfo := modelsQuery.Info() - - // count - var totalCount int64 - var baseTable string - if len(queryInfo.From) > 0 { - baseTable = queryInfo.From[0] - } - clone := modelsQuery - countQuery := clone.Distinct(false).Select("COUNT(DISTINCT [[" + baseTable + "." + s.countCol + "]])").OrderBy() - if err := countQuery.Row(&totalCount); err != nil { - return nil, err + // normalize page + if s.page <= 0 { + s.page = 1 } // normalize perPage @@ -219,31 +229,65 @@ func (s *Provider) Exec(items any) (*Result, error) { s.perPage = MaxPerPage } - totalPages := int(math.Ceil(float64(totalCount) / float64(s.perPage))) + // negative value to differentiate from the zero default + totalCount := -1 + totalPages := -1 - // normalize page according to the total count - if s.page <= 0 || totalCount == 0 { - s.page = 1 - } else if s.page > totalPages { - s.page = totalPages + // prepare a count query from the base one + countQuery := modelsQuery // shallow clone + countExec := func() error { + queryInfo := countQuery.Info() + countCol := s.countCol + if len(queryInfo.From) > 0 { + countCol = queryInfo.From[0] + "." + countCol + } + + // note: countQuery is shallow cloned and slice/map in-place modifications should be avoided + err := countQuery.Distinct(false). + Select("COUNT(DISTINCT [[" + countCol + "]])"). + OrderBy( /* reset */ ). + Row(&totalCount) + if err != nil { + return err + } + + totalPages = int(math.Ceil(float64(totalCount) / float64(s.perPage))) + + return nil } - // apply pagination - modelsQuery.Limit(int64(s.perPage)) - modelsQuery.Offset(int64(s.perPage * (s.page - 1))) + // apply pagination to the original query and fetch the models + modelsExec := func() error { + modelsQuery.Limit(int64(s.perPage)) + modelsQuery.Offset(int64(s.perPage * (s.page - 1))) - // fetch models - if err := modelsQuery.All(items); err != nil { - return nil, err + return modelsQuery.All(items) } - return &Result{ + if !s.skipTotal { + // execute the 2 queries concurrently + errg := new(errgroup.Group) + errg.SetLimit(2) + errg.Go(countExec) + errg.Go(modelsExec) + if err := errg.Wait(); err != nil { + return nil, err + } + } else { + if err := modelsExec(); err != nil { + return nil, err + } + } + + result := &Result{ Page: s.page, PerPage: s.perPage, - TotalItems: int(totalCount), + TotalItems: totalCount, TotalPages: totalPages, Items: items, - }, nil + } + + return result, nil } // ParseAndExec is a short convenient method to trigger both diff --git a/tools/search/provider_test.go b/tools/search/provider_test.go index b037b4d9..5c061888 100644 --- a/tools/search/provider_test.go +++ b/tools/search/provider_test.go @@ -42,11 +42,25 @@ func TestProviderQuery(t *testing.T) { } } +func TestProviderSkipTotal(t *testing.T) { + p := NewProvider(&testFieldResolver{}) + + if p.skipTotal { + t.Fatalf("Expected the default skipTotal to be %v, got %v", false, p.skipTotal) + } + + p.SkipTotal(true) + + if !p.skipTotal { + t.Fatalf("Expected skipTotal to change to %v, got %v", true, p.skipTotal) + } +} + func TestProviderCountCol(t *testing.T) { p := NewProvider(&testFieldResolver{}) - if p.countCol != "_rowid_" { - t.Fatalf("Expected the default countCol to be %s, got %s", "_rowid_", p.countCol) + if p.countCol != "id" { + t.Fatalf("Expected the default countCol to be %s, got %s", "id", p.countCol) } p.CountCol("test") @@ -229,6 +243,7 @@ func TestProviderExecNonEmptyQuery(t *testing.T) { perPage int sort []SortField filter []FilterData + skipTotal bool expectError bool expectResult string expectQueries []string @@ -240,23 +255,25 @@ func TestProviderExecNonEmptyQuery(t *testing.T) { []SortField{}, []FilterData{}, false, + false, `{"page":1,"perPage":10,"totalItems":2,"totalPages":1,"items":[{"test1":1,"test2":"test2.1","test3":""},{"test1":2,"test2":"test2.2","test3":""}]}`, []string{ - "SELECT COUNT(DISTINCT [[test._rowid_]]) FROM `test` WHERE NOT (`test1` IS NULL)", + "SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE NOT (`test1` IS NULL)", "SELECT * FROM `test` WHERE NOT (`test1` IS NULL) ORDER BY `test1` ASC LIMIT 10", }, }, { "perPage normalization", - 10, // will be capped by total count - 0, // fallback to default + 10, + 0, // fallback to default []SortField{}, []FilterData{}, false, - `{"page":1,"perPage":30,"totalItems":2,"totalPages":1,"items":[{"test1":1,"test2":"test2.1","test3":""},{"test1":2,"test2":"test2.2","test3":""}]}`, + false, + `{"page":10,"perPage":30,"totalItems":2,"totalPages":1,"items":[]}`, []string{ - "SELECT COUNT(DISTINCT [[test._rowid_]]) FROM `test` WHERE NOT (`test1` IS NULL)", - "SELECT * FROM `test` WHERE NOT (`test1` IS NULL) ORDER BY `test1` ASC LIMIT 30", + "SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE NOT (`test1` IS NULL)", + "SELECT * FROM `test` WHERE NOT (`test1` IS NULL) ORDER BY `test1` ASC LIMIT 30 OFFSET 270", }, }, { @@ -265,6 +282,7 @@ func TestProviderExecNonEmptyQuery(t *testing.T) { 10, []SortField{{"unknown", SortAsc}}, []FilterData{}, + false, true, "", nil, @@ -275,6 +293,7 @@ func TestProviderExecNonEmptyQuery(t *testing.T) { 10, []SortField{}, []FilterData{"test2 = 'test2.1'", "invalid"}, + false, true, "", nil, @@ -286,10 +305,24 @@ func TestProviderExecNonEmptyQuery(t *testing.T) { []SortField{{"test2", SortDesc}}, []FilterData{"test2 != null", "test1 >= 2"}, false, + false, `{"page":1,"perPage":` + fmt.Sprint(MaxPerPage) + `,"totalItems":1,"totalPages":1,"items":[{"test1":2,"test2":"test2.2","test3":""}]}`, []string{ - "SELECT COUNT(DISTINCT [[test._rowid_]]) FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 != '' AND test2 IS NOT NULL)))) AND (test1 >= 2)", - "SELECT * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 != '' AND test2 IS NOT NULL)))) AND (test1 >= 2) ORDER BY `test1` ASC, `test2` DESC LIMIT 500", + "SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 != '' AND test2 IS NOT NULL)))) AND (test1 >= 2)", + "SELECT * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 != '' AND test2 IS NOT NULL)))) AND (test1 >= 2) ORDER BY `test1` ASC, `test2` DESC LIMIT " + fmt.Sprint(MaxPerPage), + }, + }, + { + "valid sort and filter fields (skipTotal=1)", + 1, + 5555, // will be limited by MaxPerPage + []SortField{{"test2", SortDesc}}, + []FilterData{"test2 != null", "test1 >= 2"}, + true, + false, + `{"page":1,"perPage":` + fmt.Sprint(MaxPerPage) + `,"totalItems":-1,"totalPages":-1,"items":[{"test1":2,"test2":"test2.2","test3":""}]}`, + []string{ + "SELECT * FROM `test` WHERE ((NOT (`test1` IS NULL)) AND (((test2 != '' AND test2 IS NOT NULL)))) AND (test1 >= 2) ORDER BY `test1` ASC, `test2` DESC LIMIT " + fmt.Sprint(MaxPerPage), }, }, { @@ -299,22 +332,50 @@ func TestProviderExecNonEmptyQuery(t *testing.T) { []SortField{{"test3", SortAsc}}, []FilterData{"test3 != ''"}, false, + false, `{"page":1,"perPage":10,"totalItems":0,"totalPages":0,"items":[]}`, []string{ - "SELECT COUNT(DISTINCT [[test._rowid_]]) FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 != '' AND test3 IS NOT NULL)))", + "SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 != '' AND test3 IS NOT NULL)))", + "SELECT * FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 != '' AND test3 IS NOT NULL))) ORDER BY `test1` ASC, `test3` ASC LIMIT 10", + }, + }, + { + "valid sort and filter fields (zero results; skipTotal=1)", + 1, + 10, + []SortField{{"test3", SortAsc}}, + []FilterData{"test3 != ''"}, + true, + false, + `{"page":1,"perPage":10,"totalItems":-1,"totalPages":-1,"items":[]}`, + []string{ "SELECT * FROM `test` WHERE (NOT (`test1` IS NULL)) AND (((test3 != '' AND test3 IS NOT NULL))) ORDER BY `test1` ASC, `test3` ASC LIMIT 10", }, }, { "pagination test", - 3, + 2, 1, []SortField{}, []FilterData{}, false, + false, `{"page":2,"perPage":1,"totalItems":2,"totalPages":2,"items":[{"test1":2,"test2":"test2.2","test3":""}]}`, []string{ - "SELECT COUNT(DISTINCT [[test._rowid_]]) FROM `test` WHERE NOT (`test1` IS NULL)", + "SELECT COUNT(DISTINCT [[test.id]]) FROM `test` WHERE NOT (`test1` IS NULL)", + "SELECT * FROM `test` WHERE NOT (`test1` IS NULL) ORDER BY `test1` ASC LIMIT 1 OFFSET 1", + }, + }, + { + "pagination test (skipTotal=1)", + 2, + 1, + []SortField{}, + []FilterData{}, + true, + false, + `{"page":2,"perPage":1,"totalItems":-1,"totalPages":-1,"items":[{"test1":2,"test2":"test2.2","test3":""}]}`, + []string{ "SELECT * FROM `test` WHERE NOT (`test1` IS NULL) ORDER BY `test1` ASC LIMIT 1 OFFSET 1", }, }, @@ -329,6 +390,7 @@ func TestProviderExecNonEmptyQuery(t *testing.T) { Page(s.page). PerPage(s.perPage). Sort(s.sort). + SkipTotal(s.skipTotal). Filter(s.filter) result, err := p.Exec(&[]testTableStruct{}) @@ -378,55 +440,74 @@ func TestProviderParseAndExec(t *testing.T) { OrderBy("test1 ASC") scenarios := []struct { + name string queryString string expectError bool expectResult string }{ - // empty { + "no extra query params (aka. use the provider presets)", "", false, - `{"page":1,"perPage":123,"totalItems":2,"totalPages":1,"items":[{"test1":1,"test2":"test2.1","test3":""},{"test1":2,"test2":"test2.2","test3":""}]}`, + `{"page":2,"perPage":123,"totalItems":2,"totalPages":1,"items":[]}`, }, - // invalid query { + "invalid query", "invalid;", true, "", }, - // invalid page { + "invalid page", "page=a", true, "", }, - // invalid perPage { + "invalid perPage", "perPage=a", true, "", }, - // invalid sorting field { + "invalid skipTotal", + "skipTotal=a", + true, + "", + }, + { + "invalid sorting field", "sort=-unknown", true, "", }, - // invalid filter field { + "invalid filter field", "filter=unknown>1", true, "", }, - // valid query params { - "page=3&perPage=9999&filter=test1>1&sort=-test2,test3", + "page > existing", + "page=3&perPage=9999", false, - `{"page":1,"perPage":500,"totalItems":1,"totalPages":1,"items":[{"test1":2,"test2":"test2.2","test3":""}]}`, + `{"page":3,"perPage":1000,"totalItems":2,"totalPages":1,"items":[]}`, + }, + { + "valid query params", + "page=1&perPage=9999&filter=test1>1&sort=-test2,test3", + false, + `{"page":1,"perPage":1000,"totalItems":1,"totalPages":1,"items":[{"test1":2,"test2":"test2.2","test3":""}]}`, + }, + { + "valid query params with skipTotal=1", + "page=1&perPage=9999&filter=test1>1&sort=-test2,test3&skipTotal=1", + false, + `{"page":1,"perPage":1000,"totalItems":-1,"totalPages":-1,"items":[{"test1":2,"test2":"test2.2","test3":""}]}`, }, } - for i, s := range scenarios { + for _, s := range scenarios { testDB.CalledQueries = []string{} // reset testResolver := &testFieldResolver{} @@ -441,7 +522,7 @@ func TestProviderParseAndExec(t *testing.T) { hasErr := err != nil if hasErr != s.expectError { - t.Errorf("(%d) Expected hasErr %v, got %v (%v)", i, s.expectError, hasErr, err) + t.Errorf("[%s] Expected hasErr %v, got %v (%v)", s.name, s.expectError, hasErr, err) continue } @@ -450,16 +531,21 @@ func TestProviderParseAndExec(t *testing.T) { } if testResolver.UpdateQueryCalls != 1 { - t.Errorf("(%d) Expected resolver.Update to be called %d, got %d", i, 1, testResolver.UpdateQueryCalls) + t.Errorf("[%s] Expected resolver.Update to be called %d, got %d", s.name, 1, testResolver.UpdateQueryCalls) } - if len(testDB.CalledQueries) != 2 { - t.Errorf("(%d) Expected %d db queries, got %d: \n%v", i, 2, len(testDB.CalledQueries), testDB.CalledQueries) + expectedQueries := 2 + if provider.skipTotal { + expectedQueries = 1 + } + + if len(testDB.CalledQueries) != expectedQueries { + t.Errorf("[%s] Expected %d db queries, got %d: \n%v", s.name, expectedQueries, len(testDB.CalledQueries), testDB.CalledQueries) } encoded, _ := json.Marshal(result) if string(encoded) != s.expectResult { - t.Errorf("(%d) Expected result %v, got \n%v", i, s.expectResult, string(encoded)) + t.Errorf("[%s] Expected result %v, got \n%v", s.name, s.expectResult, string(encoded)) } } } @@ -481,7 +567,9 @@ type testDB struct { // NB! Don't forget to call `db.Close()` at the end of the test. func createTestDB() (*testDB, error) { - sqlDB, err := sql.Open("sqlite", ":memory:") + // using a shared cache to allow multiple connections access to + // the same in memory database https://www.sqlite.org/inmemorydb.html + sqlDB, err := sql.Open("sqlite", "file::memory:?cache=shared") if err != nil { return nil, err }