fix: improve query performances (v1.10.x) (#447)

* fix: improve query performances * ci: fix workflow * ci: fix workflow --------- Co-authored-by: Maxence Maireaux <maxence@maireaux.fr>
formancehq · Jun 15, 2023 · 34b5998 · 34b5998
1 parent 3866243
commit 34b5998
Show file tree

Hide file tree

Showing 10 changed files with 170 additions and 40 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -13,6 +13,30 @@ jobs:
     if: github.event_name == 'pull_request'
     uses: formancehq/gh-workflows/.github/workflows/pr-style.yml@main
 
+  Test_postgres:
+    name: 'Test with PostgreSQL'
+    runs-on: ubuntu-latest
+    steps:
+      - name: Install task
+        uses: arduino/setup-task@v1
+        with:
+          repo-token: ${{ secrets.GITHUB_TOKEN }}
+      - uses: actions/checkout@v3
+      - uses: actions/setup-go@v3
+        with:
+          go-version-file: 'go.mod'
+          cache: true
+      - name: Run tests
+        run: task tests
+      - name: Upload coverage to Codecov
+        uses: codecov/codecov-action@v3
+        with:
+          fail_ci_if_error: false # optional (default = false)
+          verbose: true # optional (default = false)
+
+  Test_sqlite:
+    uses: formancehq/gh-workflows/.github/workflows/golang-test.yml@main
+
   Control:
     name: 'Control'
     uses: ./.github/workflows/template_build-control.yaml
@@ -22,6 +46,8 @@ jobs:
   GoReleaserBuild:
     needs:
       - Control
+      - Test_sqlite
+      - Test_postgres
     if: github.event_name != 'release'
     name: 'GoReleaser Build'
     uses: ./.github/workflows/template_goreleaser-build.yaml
@@ -32,6 +58,8 @@ jobs:
   GoReleaserRelease:
     needs:
       - Control
+      - Test_sqlite
+      - Test_postgres
     if: github.event_name == 'release'
     name: 'GoReleaser Release'
     uses: ./.github/workflows/template_goreleaser-release.yaml
@@ -59,6 +87,8 @@ jobs:
   DockerRelease:
     needs:
       - Control
+      - Test_sqlite
+      - Test_postgres
     if: github.event_name == 'release'
     uses: ./.github/workflows/template_docker.yaml
     with:
@@ -72,6 +102,8 @@ jobs:
   DockerBranch:
     needs:
       - Control
+      - Test_sqlite
+      - Test_postgres
     if: github.event_name != 'release'
     uses: ./.github/workflows/template_docker.yaml
     with:

diff --git a/Taskfile.yaml b/Taskfile.yaml
@@ -135,7 +135,7 @@ tasks:
         msg: Please specify generator as first cli arg (ie "task template -- go")
     cmds:
       - >
-        docker run --rm -w /local -v ${PWD}:/local openapitools/openapi-generator-cli:v6.1.0 author
+        docker run --rm -w /local -v ${PWD}:/local openapitools/openapi-generator-cli:v6.4.0 author
         template -g {{.CLI_ARGS}} -o templates/{{.CLI_ARGS}}
 
   sdk:generate:
@@ -145,10 +145,10 @@ tasks:
       - sh: '[ "{{.CLI_ARGS}}" != "" ]'
         msg: Please specify generator as first cli arg (ie "task generate -- go")
     cmds:
-      - wget https://raw.githubusercontent.com/formancehq/ledger/{{.VERSION}}/pkg/api/controllers/swagger.yaml -O swagger.yaml
+      - cp ../pkg/api/controllers/swagger.yaml swagger.yaml
       - sed -i -e "s/LEDGER_VERSION/{{.VERSION}}/g" swagger.yaml
       - >
-        docker run --rm -w /local -v ${PWD}:/local openapitools/openapi-generator-cli:v6.1.0 generate
+        docker run --rm -w /local -v ${PWD}:/local openapitools/openapi-generator-cli:v6.4.0 generate
         -i ./swagger.yaml
         -g {{ (split "-" .CLI_ARGS)._0 }}
         -c ./configs/{{.CLI_ARGS}}.yaml

diff --git a/pkg/api/controllers/pagination_test.go b/pkg/api/controllers/pagination_test.go
@@ -471,7 +471,7 @@ func TestCursor(t *testing.T) {
 				t.Run("GetAccounts", func(t *testing.T) {
 					httpResponse := internal.GetAccounts(api, url.Values{
 						"after":                             []string{"accounts:15"},
-						"address":                           []string{"acc.*"},
+						"address":                           []string{"accounts:.*"},
 						"metadata[foo]":                     []string{"bar"},
 						"balance":                           []string{"1"},
 						controllers.QueryKeyBalanceOperator: []string{"gte"},
@@ -483,7 +483,7 @@ func TestCursor(t *testing.T) {
 					res, err := base64.RawURLEncoding.DecodeString(cursor.Next)
 					require.NoError(t, err)
 					require.Equal(t,
-						`{"pageSize":3,"offset":3,"after":"accounts:15","address":"acc.*","metadata":{"foo":"bar"},"balance":"1","balanceOperator":"gte"}`,
+						`{"pageSize":3,"offset":3,"after":"accounts:15","address":"accounts:.*","metadata":{"foo":"bar"},"balance":"1","balanceOperator":"gte"}`,
 						string(res))
 
 					httpResponse = internal.GetAccounts(api, url.Values{
@@ -495,12 +495,12 @@ func TestCursor(t *testing.T) {
 					res, err = base64.RawURLEncoding.DecodeString(cursor.Previous)
 					require.NoError(t, err)
 					require.Equal(t,
-						`{"pageSize":3,"offset":0,"after":"accounts:15","address":"acc.*","metadata":{"foo":"bar"},"balance":"1","balanceOperator":"gte"}`,
+						`{"pageSize":3,"offset":0,"after":"accounts:15","address":"accounts:.*","metadata":{"foo":"bar"},"balance":"1","balanceOperator":"gte"}`,
 						string(res))
 					res, err = base64.RawURLEncoding.DecodeString(cursor.Next)
 					require.NoError(t, err)
 					require.Equal(t,
-						`{"pageSize":3,"offset":6,"after":"accounts:15","address":"acc.*","metadata":{"foo":"bar"},"balance":"1","balanceOperator":"gte"}`,
+						`{"pageSize":3,"offset":6,"after":"accounts:15","address":"accounts:.*","metadata":{"foo":"bar"},"balance":"1","balanceOperator":"gte"}`,
 						string(res))
 				})
 
@@ -545,7 +545,7 @@ func TestCursor(t *testing.T) {
 				t.Run("GetBalances", func(t *testing.T) {
 					httpResponse := internal.GetBalances(api, url.Values{
 						"after":                      []string{"accounts:15"},
-						"address":                    []string{"acc.*"},
+						"address":                    []string{"accounts:.*"},
 						controllers.QueryKeyPageSize: []string{"3"},
 					})
 					assert.Equal(t, http.StatusOK, httpResponse.Result().StatusCode, httpResponse.Body.String())
@@ -554,7 +554,7 @@ func TestCursor(t *testing.T) {
 					res, err := base64.RawURLEncoding.DecodeString(cursor.Next)
 					require.NoError(t, err)
 					require.Equal(t,
-						`{"pageSize":3,"offset":3,"after":"accounts:15","address":"acc.*"}`,
+						`{"pageSize":3,"offset":3,"after":"accounts:15","address":"accounts:.*"}`,
 						string(res))
 
 					httpResponse = internal.GetBalances(api, url.Values{
@@ -566,12 +566,12 @@ func TestCursor(t *testing.T) {
 					res, err = base64.RawURLEncoding.DecodeString(cursor.Previous)
 					require.NoError(t, err)
 					require.Equal(t,
-						`{"pageSize":3,"offset":0,"after":"accounts:15","address":"acc.*"}`,
+						`{"pageSize":3,"offset":0,"after":"accounts:15","address":"accounts:.*"}`,
 						string(res))
 					res, err = base64.RawURLEncoding.DecodeString(cursor.Next)
 					require.NoError(t, err)
 					require.Equal(t,
-						`{"pageSize":3,"offset":6,"after":"accounts:15","address":"acc.*"}`,
+						`{"pageSize":3,"offset":6,"after":"accounts:15","address":"accounts:.*"}`,
 						string(res))
 				})
 

diff --git a/pkg/storage/sqlstorage/accounts.go b/pkg/storage/sqlstorage/accounts.go
@@ -6,6 +6,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -17,6 +18,11 @@ import (
 	"github.com/pkg/errors"
 )
 
+// This regexp is used to validate the account name
+// If the account name is not valid, it means that the user putted a regex in
+// the address filter, and we have to change the postgres operator used.
+var accountNameRegex = regexp.MustCompile(`^[a-zA-Z_0-9]+$`)
+
 func (s *Store) buildAccountsQuery(p ledger.AccountsQuery) (*sqlbuilder.SelectBuilder, AccPaginationToken) {
 	sb := sqlbuilder.NewSelectBuilder()
 	t := AccPaginationToken{}
@@ -30,11 +36,26 @@ func (s *Store) buildAccountsQuery(p ledger.AccountsQuery) (*sqlbuilder.SelectBu
 	)
 
 	if address != "" {
-		arg := sb.Args.Add("^" + address + "$")
 		switch s.Schema().Flavor() {
 		case sqlbuilder.PostgreSQL:
-			sb.Where("address ~* " + arg)
+			src := strings.Split(address, ":")
+			sb.Where(fmt.Sprintf("jsonb_array_length(address_json) = %d", len(src)))
+
+			for i, segment := range src {
+				if segment == ".*" || segment == "*" || segment == "" {
+					continue
+				}
+
+				operator := "=="
+				if !accountNameRegex.MatchString(segment) {
+					operator = "like_regex"
+				}
+
+				arg := sb.Args.Add(segment)
+				sb.Where(fmt.Sprintf("address_json @@ ('$[%d] %s \"' || %s::text || '\"')::jsonpath", i, operator, arg))
+			}
 		case sqlbuilder.SQLite:
+			arg := sb.Args.Add("^" + address + "$")
 			sb.Where("address REGEXP " + arg)
 		}
 		t.AddressRegexpFilter = address
@@ -206,14 +227,33 @@ func (s *Store) GetAccount(ctx context.Context, addr string) (*core.Account, err
 }
 
 func (s *Store) ensureAccountExists(ctx context.Context, account string) error {
+	var accountBy string
+	switch s.schema.Flavor() {
+	case sqlbuilder.PostgreSQL:
+		a, err := json.Marshal(strings.Split(account, ":"))
+		if err != nil {
+			return err
+		}
+		accountBy = string(a)
+	case sqlbuilder.SQLite:
+		accountBy = account
+	}
 
 	sb := sqlbuilder.NewInsertBuilder()
-	sqlq, args := sb.
-		InsertInto(s.schema.Table("accounts")).
-		Cols("address", "metadata").
-		Values(account, "{}").
-		SQL("ON CONFLICT DO NOTHING").
-		BuildWithFlavor(s.schema.Flavor())
+	sb = sb.InsertInto(s.schema.Table("accounts"))
+
+	switch s.schema.Flavor() {
+	case sqlbuilder.PostgreSQL:
+		sb = sb.Cols("address", "metadata", "address_json").
+			Values(account, "{}", accountBy).
+			SQL("ON CONFLICT DO NOTHING")
+	case sqlbuilder.SQLite:
+		sb = sb.Cols("address", "metadata").
+			Values(account, "{}").
+			SQL("ON CONFLICT DO NOTHING")
+	}
+
+	sqlq, args := sb.BuildWithFlavor(s.schema.Flavor())
 
 	executor, err := s.executorProvider(ctx)
 	if err != nil {
@@ -231,17 +271,23 @@ func (s *Store) UpdateAccountMetadata(ctx context.Context, address string, metad
 	if err != nil {
 		return err
 	}
+
 	placeholder := ib.Var(metadataData)
-	ib.
-		InsertInto(s.schema.Table("accounts")).
-		Cols("address", "metadata").
-		Values(address, metadataData)
-
-	switch Flavor(s.schema.Flavor()) {
-	case PostgreSQL:
-		ib.SQL(fmt.Sprintf("ON CONFLICT (address) DO UPDATE SET metadata = accounts.metadata || %s", placeholder))
-	case SQLite:
-		ib.SQL(fmt.Sprintf("ON CONFLICT (address) DO UPDATE SET metadata = json_patch(metadata,  %s)", placeholder))
+	ib = ib.InsertInto(s.schema.Table("accounts"))
+
+	switch s.schema.Flavor() {
+	case sqlbuilder.PostgreSQL:
+		addressBy, err := json.Marshal(strings.Split(address, ":"))
+		if err != nil {
+			return err
+		}
+		ib = ib.Cols("address", "metadata", "address_json").
+			Values(address, metadataData, addressBy).
+			SQL(fmt.Sprintf("ON CONFLICT (address) DO UPDATE SET metadata = accounts.metadata || %s", placeholder))
+	case sqlbuilder.SQLite:
+		ib = ib.Cols("address", "metadata").
+			Values(address, metadataData).
+			SQL(fmt.Sprintf("ON CONFLICT (address) DO UPDATE SET metadata = json_patch(metadata,  %s)", placeholder))
 	}
 
 	executor, err := s.executorProvider(ctx)

diff --git a/pkg/storage/sqlstorage/balances.go b/pkg/storage/sqlstorage/balances.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"strconv"
 	"strings"
 
@@ -21,11 +22,26 @@ func (s *Store) GetBalancesAggregated(ctx context.Context, q ledger.BalancesQuer
 	sb.GroupBy("asset")
 
 	if q.Filters.AddressRegexp != "" {
-		arg := sb.Args.Add("^" + q.Filters.AddressRegexp + "$")
 		switch s.Schema().Flavor() {
 		case sqlbuilder.PostgreSQL:
-			sb.Where("account ~* " + arg)
+			src := strings.Split(q.Filters.AddressRegexp, ":")
+			sb.Where(fmt.Sprintf("jsonb_array_length(account_json) = %d", len(src)))
+
+			for i, segment := range src {
+				if segment == ".*" || segment == "*" || segment == "" {
+					continue
+				}
+
+				operator := "=="
+				if !accountNameRegex.MatchString(segment) {
+					operator = "like_regex"
+				}
+
+				arg := sb.Args.Add(segment)
+				sb.Where(fmt.Sprintf("account_json @@ ('$[%d] %s \"' || %s::text || '\"')::jsonpath", i, operator, arg))
+			}
 		case sqlbuilder.SQLite:
+			arg := sb.Args.Add("^" + q.Filters.AddressRegexp + "$")
 			sb.Where("account REGEXP " + arg)
 		}
 	}
@@ -96,11 +112,21 @@ func (s *Store) GetBalances(ctx context.Context, q ledger.BalancesQuery) (api.Cu
 	}
 
 	if q.Filters.AddressRegexp != "" {
-		arg := sb.Args.Add("^" + q.Filters.AddressRegexp + "$")
 		switch s.Schema().Flavor() {
 		case sqlbuilder.PostgreSQL:
-			sb.Where("account ~* " + arg)
+			src := strings.Split(q.Filters.AddressRegexp, ":")
+			sb.Where(fmt.Sprintf("jsonb_array_length(account_json) = %d", len(src)))
+
+			for i, segment := range src {
+				if segment == ".*" || segment == "*" || segment == "" {
+					continue
+				}
+
+				arg := sb.Args.Add(segment)
+				sb.Where(fmt.Sprintf("account_json @@ ('$[%d] like_regex \"' || %s::text || '\"')::jsonpath", i, arg))
+			}
 		case sqlbuilder.SQLite:
+			arg := sb.Args.Add("^" + q.Filters.AddressRegexp + "$")
 			sb.Where("account REGEXP " + arg)
 		}
 		t.AddressRegexpFilter = q.Filters.AddressRegexp

diff --git a/pkg/storage/sqlstorage/balances_test.go b/pkg/storage/sqlstorage/balances_test.go
@@ -114,7 +114,7 @@ func testGetBalances(t *testing.T, store *sqlstorage.Store) {
 			ledger.BalancesQuery{
 				PageSize:     10,
 				AfterAddress: "world",
-				Filters:      ledger.BalancesQueryFilters{AddressRegexp: "users.+"},
+				Filters:      ledger.BalancesQueryFilters{AddressRegexp: "users:.+"},
 			})
 		assert.NoError(t, err)
 		assert.Equal(t, 10, cursor.PageSize)

diff --git a/pkg/storage/sqlstorage/migrates/18-optimized-accounts/postgres.sql b/pkg/storage/sqlstorage/migrates/18-optimized-accounts/postgres.sql
@@ -0,0 +1,6 @@
+--statement
+alter table "VAR_LEDGER_NAME".accounts add column if not exists address_json jsonb;
+--statement
+UPDATE "VAR_LEDGER_NAME".accounts SET address_json = to_jsonb(string_to_array(address, ':'));
+--statement
+create index if not exists accounts_address_json on "VAR_LEDGER_NAME".accounts using GIN(address_json);
diff --git a/pkg/storage/sqlstorage/migrates/19-optimized-volumes/postgres.sql b/pkg/storage/sqlstorage/migrates/19-optimized-volumes/postgres.sql
@@ -0,0 +1,6 @@
+--statement
+alter table "VAR_LEDGER_NAME".volumes add column if not exists account_json jsonb;
+--statement
+UPDATE "VAR_LEDGER_NAME".volumes SET account_json = to_jsonb(string_to_array(account, ':'));
+--statement
+create index if not exists volumes_account_json on "VAR_LEDGER_NAME".volumes using GIN(account_json);
diff --git a/pkg/storage/sqlstorage/store_ledger_test.go b/pkg/storage/sqlstorage/store_ledger_test.go
@@ -450,7 +450,7 @@ func testGetAccounts(t *testing.T, store *sqlstorage.Store) {
 	accounts, err = store.GetAccounts(context.Background(), ledger.AccountsQuery{
 		PageSize: 10,
 		Filters: ledger.AccountsQueryFilters{
-			Address: ".*der.*",
+			Address: `^.*der:.*$`,
 		},
 	})
 	require.NoError(t, err)