api: allow filter EVM tokens by name #460

pro-wh · 2023-06-24T00:06:49Z

tested locally on ~500 blocks from Emerald. very few tokens though

pro-wh · 2023-06-24T00:08:32Z

storage/migrations/08_evm_tokens_name_index.up.sql

+
+CREATE EXTENSION pg_trgm;
+
+CREATE INDEX ix_evm_tokens ON chain.evm_tokens USING GIST (token_name gist_trgm_ops);


https://www.postgresql.org/docs/current/pgtrgm.html

can someone check if this is right

oh the index name was not good. fixed to be more descriptive

pro-wh · 2023-06-24T00:09:01Z

storage/client/queries/queries.go

@@ -462,10 +462,11 @@ const (
 		WHERE
 			(tokens.runtime = $1) AND
 			($2::oasis_addr IS NULL OR tokens.token_address = $2::oasis_addr) AND
+			($3::text IS NULL OR tokens.token_name ILIKE '%' || $3 || '%') AND


the name input is not escaped, searches with % and _ will do special things

The issue also mentioned filtering by ticker (symbol in the db).

oh let me add that

added same stuff for symbol

pro-wh · 2023-06-26T23:16:29Z

storage/migrations/08_evm_tokens_name_index.up.sql

+
+BEGIN;
+
+CREATE EXTENSION pg_trgm;


this breaks our wipe db script 💀

added some code to drop extensions too 💀 💀

You could also just go with CREATE EXTENSION IF NOT EXISTS pg_trgm, and avoid the wiping. I'd prefer that, actually, for simplicity. It's extremely unlikely that we'll want to redefine the extension at some point :)

Alternatively, I think it would also not be irresponsible to just omit the index. We have 2k EVM tokens in mainnet emerald. Even at 10k, scanning through their short names should be very fast.

Edit: I tried in prod mainnet now, with and without the index. The index does get used. Make of the time difference what you will:

[0/21466] oasisindexer=> explain analyze select count(*) from chain.evm_tokens where token_name ilike '%eth%'; QUERY PLAN ------------------------------------------------------------------------------------------------------------- Aggregate (cost=68.17..68.18 rows=1 width=8) (actual time=1.491..1.493 rows=1 loops=1) -> Seq Scan on evm_tokens (cost=0.00..68.04 rows=53 width=0) (actual time=0.016..1.481 rows=58 loops=1) Filter: (token_name ~~* '%eth%'::text) Rows Removed by Filter: 1865 Planning Time: 0.325 ms Execution Time: 1.544 ms (6 rows) oasisindexer=> CREATE EXTENSION pg_trgm; CREATE EXTENSION oasisindexer=> oasisindexer=> CREATE INDEX ix_evm_tokens_name ON chain.evm_tokens USING GIST (token_name gist_trgm_ops); CREATE INDEX oasisindexer=> CREATE INDEX ix_evm_tokens_symbol ON chain.evm_tokens USING GIST (symbol gist_trgm_ops); CREATE INDEX oasisindexer=> explain analyze select count(*) from chain.evm_tokens where token_name ilike '%eth%'; QUERY PLAN ----------------------------------------------------------------------------------------------------------------------------------- Aggregate (cost=51.69..51.70 rows=1 width=8) (actual time=0.455..0.456 rows=1 loops=1) -> Bitmap Heap Scan on evm_tokens (cost=4.56..51.56 rows=53 width=0) (actual time=0.360..0.447 rows=58 loops=1) Recheck Cond: (token_name ~~* '%eth%'::text) Heap Blocks: exact=30 -> Bitmap Index Scan on ix_evm_tokens_name (cost=0.00..4.54 rows=53 width=0) (actual time=0.344..0.345 rows=58 loops=1) Index Cond: (token_name ~~* '%eth%'::text) Planning Time: 0.538 ms Execution Time: 0.651 ms

on 58 entries?

well I'll move this to a separate PR, let's hold off until it becomes slower

this query is going to run ~on keystroke, so

oh but

{"args":[],"caller":"client.go:60","db":"indexer","err":"ERROR: cannot drop type gtrgm because extension pg_trgm requires it (SQLSTATE 2BP01)","level":"error","module":"postgres","msg":"Query","pid":188,"sql":"DROP TYPE public.gtrgm CASCADE;","time":"158.785µs","ts":"2023-06-27T18:11:29.493112027Z"}

with the current wipe code

We can manually exclude gtrgm from the list of types to be wiped? It's a little hairy but oh well, the whole Wipe() concept is vaguely a hack. I checked just now and it seems to be the only type created by the extension.

on 58 entries?
58 that match '%eth%', but ~2k entries total (Rows Removed by Filter: 1865)

then there's a bunch of functions that it depends on

pro-wh · 2023-06-26T23:30:34Z

storage/postgres/client.go

+	if err != nil {
+		return fmt.Errorf("failed to list extensions: %w", err)
+	}
+	defer rows.Close()


we use multiple defer rows.Close() in this function, reassigning rows 💀 💀 💀

I think defer captures enough context that it's all OK? Example: https://go.dev/play/p/6g8xQemALDi

If not, please do fix 🙏

struct, pointer receiver
https://go.dev/play/p/fWui7UAvo7N -> 2 2 ❌

pointer to struct, pointer receiver
https://go.dev/play/p/FNsKXdCQ7Fs -> 2 1 ✔️

interface with pointer to struct, pointer receiver
https://go.dev/play/p/Nywd7MMYyvd -> 2 1 ✔️

interface with struct, struct receiver
https://go.dev/play/p/E4f6lT8zO5o -> 2 1 ✔️

this stuff is too... it's like the phrase "sensitive to initial conditions" that you might say of a physical system. I'm just gonna wrap these in iifes.

although I'm going to get rid of this and go with the create extension if not exists way

hm there's a listIndexerTables in its own method, so I'll go with methods

pro-wh · 2023-06-26T23:31:16Z

storage/postgres/client.go

+			return err
+		}
+		// This one was here when I got here.
+		if extension == "plpgsql" {


never heard of this extension before, so not touching it??? 💀 💀 💀 💀

I'm surprised this is even an extension, technically. It's the de-facto default procedural language for writing functions in postgres. And yes, good call not to remove, I've used it in ad-hoc ways before, and we might use it later.

WDYT about avoiding this if and just doing SELECT ... WHERE extname != 'plpgsql' instead? It's how most other filtering in Wipe() is done.

oh ok

although let's go with the create extension if not exists

pro-wh · 2023-06-26T23:32:37Z

rebased on main

pro-wh · 2023-06-26T23:33:30Z

api/spec/v1.yaml

+          name: name
+          schema:
+            type: string
+          description: A filter on the name, the name must contain this value as a substring.


ugh this should be updated to say that the symbol can match too

mitjat

Thanks! I have some ideas for simplifying, see what you think.

mitjat · 2023-06-27T08:02:40Z

storage/postgres/client.go

+			return err
+		}
+		// This one was here when I got here.
+		if extension == "plpgsql" {


I'm surprised this is even an extension, technically. It's the de-facto default procedural language for writing functions in postgres. And yes, good call not to remove, I've used it in ad-hoc ways before, and we might use it later.

WDYT about avoiding this if and just doing SELECT ... WHERE extname != 'plpgsql' instead? It's how most other filtering in Wipe() is done.

mitjat · 2023-06-27T08:06:07Z

storage/postgres/client.go

+	if err != nil {
+		return fmt.Errorf("failed to list extensions: %w", err)
+	}
+	defer rows.Close()


I think defer captures enough context that it's all OK? Example: https://go.dev/play/p/6g8xQemALDi

If not, please do fix 🙏

mitjat · 2023-06-27T08:07:49Z

storage/migrations/08_evm_tokens_name_index.up.sql

+
+BEGIN;
+
+CREATE EXTENSION pg_trgm;


You could also just go with CREATE EXTENSION IF NOT EXISTS pg_trgm, and avoid the wiping. I'd prefer that, actually, for simplicity. It's extremely unlikely that we'll want to redefine the extension at some point :)

mitjat · 2023-06-27T08:18:07Z

storage/migrations/08_evm_tokens_name_index.up.sql

+
+BEGIN;
+
+CREATE EXTENSION pg_trgm;


Alternatively, I think it would also not be irresponsible to just omit the index. We have 2k EVM tokens in mainnet emerald. Even at 10k, scanning through their short names should be very fast.

Edit: I tried in prod mainnet now, with and without the index. The index does get used. Make of the time difference what you will:

[0/21466] oasisindexer=> explain analyze select count(*) from chain.evm_tokens where token_name ilike '%eth%'; QUERY PLAN ------------------------------------------------------------------------------------------------------------- Aggregate (cost=68.17..68.18 rows=1 width=8) (actual time=1.491..1.493 rows=1 loops=1) -> Seq Scan on evm_tokens (cost=0.00..68.04 rows=53 width=0) (actual time=0.016..1.481 rows=58 loops=1) Filter: (token_name ~~* '%eth%'::text) Rows Removed by Filter: 1865 Planning Time: 0.325 ms Execution Time: 1.544 ms (6 rows) oasisindexer=> CREATE EXTENSION pg_trgm; CREATE EXTENSION oasisindexer=> oasisindexer=> CREATE INDEX ix_evm_tokens_name ON chain.evm_tokens USING GIST (token_name gist_trgm_ops); CREATE INDEX oasisindexer=> CREATE INDEX ix_evm_tokens_symbol ON chain.evm_tokens USING GIST (symbol gist_trgm_ops); CREATE INDEX oasisindexer=> explain analyze select count(*) from chain.evm_tokens where token_name ilike '%eth%'; QUERY PLAN ----------------------------------------------------------------------------------------------------------------------------------- Aggregate (cost=51.69..51.70 rows=1 width=8) (actual time=0.455..0.456 rows=1 loops=1) -> Bitmap Heap Scan on evm_tokens (cost=4.56..51.56 rows=53 width=0) (actual time=0.360..0.447 rows=58 loops=1) Recheck Cond: (token_name ~~* '%eth%'::text) Heap Blocks: exact=30 -> Bitmap Index Scan on ix_evm_tokens_name (cost=0.00..4.54 rows=53 width=0) (actual time=0.344..0.345 rows=58 loops=1) Index Cond: (token_name ~~* '%eth%'::text) Planning Time: 0.538 ms Execution Time: 0.651 ms

pro-wh requested review from aefhm, Andrew7234, mitjat and ptrus as code owners June 24, 2023 00:06

pro-wh commented Jun 24, 2023

View reviewed changes

pro-wh commented Jun 26, 2023

View reviewed changes

pro-wh force-pushed the pro-wh/feature/tokenname branch from 7bda4f6 to 78e65b1 Compare June 26, 2023 23:29

pro-wh commented Jun 26, 2023

View reviewed changes

pro-wh changed the base branch from pro-wh/feature/holders10 to main June 26, 2023 23:32

pro-wh commented Jun 26, 2023

View reviewed changes

mitjat approved these changes Jun 27, 2023

View reviewed changes

api: allow filter EVM tokens by name

f29b2db

pro-wh force-pushed the pro-wh/feature/tokenname branch from 78e65b1 to f29b2db Compare June 27, 2023 21:03

pro-wh merged commit beeaf1c into main Jun 27, 2023

pro-wh deleted the pro-wh/feature/tokenname branch June 27, 2023 21:38

pro-wh mentioned this pull request Jun 27, 2023

storage: add chain.evm_tokens token_name index #466

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: allow filter EVM tokens by name #460

api: allow filter EVM tokens by name #460

pro-wh commented Jun 24, 2023 •

edited

Loading

pro-wh Jun 24, 2023

pro-wh Jun 26, 2023

pro-wh Jun 24, 2023

ptrus Jun 26, 2023 •

edited by csillag

Loading

pro-wh Jun 26, 2023

pro-wh Jun 26, 2023

pro-wh Jun 26, 2023

pro-wh Jun 26, 2023

mitjat Jun 27, 2023

mitjat Jun 27, 2023

pro-wh Jun 27, 2023

pro-wh Jun 27, 2023 •

edited

Loading

mitjat Jun 27, 2023 •

edited

Loading

pro-wh Jun 27, 2023

pro-wh Jun 26, 2023

mitjat Jun 27, 2023

pro-wh Jun 27, 2023

pro-wh Jun 27, 2023

pro-wh Jun 27, 2023

pro-wh Jun 27, 2023

pro-wh Jun 26, 2023

mitjat Jun 27, 2023

pro-wh Jun 27, 2023

pro-wh commented Jun 26, 2023

pro-wh Jun 26, 2023

mitjat left a comment

mitjat Jun 27, 2023

mitjat Jun 27, 2023

mitjat Jun 27, 2023

mitjat Jun 27, 2023


		CREATE EXTENSION pg_trgm;

		CREATE INDEX ix_evm_tokens ON chain.evm_tokens USING GIST (token_name gist_trgm_ops);

api: allow filter EVM tokens by name #460

api: allow filter EVM tokens by name #460

Conversation

pro-wh commented Jun 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ptrus Jun 26, 2023 • edited by csillag Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pro-wh Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

mitjat Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pro-wh commented Jun 26, 2023

Choose a reason for hiding this comment

mitjat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pro-wh commented Jun 24, 2023 •

edited

Loading

ptrus Jun 26, 2023 •

edited by csillag

Loading

pro-wh Jun 27, 2023 •

edited

Loading

mitjat Jun 27, 2023 •

edited

Loading