Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: tx list function name fuzzy search #2102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
He1DAr wants to merge 1 commit into develop
base: develop
Choose a base branch
Loading
from feat/tx-list-function-name-fuzzy-search

Conversation

Copy link
Contributor

@He1DAr He1DAr commented Oct 1, 2024
edited
Loading

Implemented fuzzy search for transaction list query using the pg_trgm extension, starting with indexing the contract_call_function_name column. Future plan is to expand the index to cover more columns, improving search flexibility, e.g.:

CREATE INDEX idx_search_term_trgm
ON public.txs
USING gin (
 contract_call_function_name gin_trgm_ops,
 contract_name gin_trgm_ops
 memo gin_trgm_ops
 ....
);

Copy link

github-actions bot commented Oct 1, 2024
edited
Loading

Copy link

codecov bot commented Oct 1, 2024
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@He1DAr He1DAr marked this pull request as draft October 1, 2024 22:25
Comment on lines +116 to +121
search_term: Type.Optional(
Type.String({
description: 'Option to search for transactions by a search term',
examples: ['swap'],
})
),
Copy link
Contributor

@zone117x zone117x Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also be paired with something like a search_field_name, which current supports contract_call_function_name? Seems like there will be use cases were a search will not be interested in every single supported field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to have the search_term as a broad search filter that looks into multiple fields. My plan so far is to incrementally add more fields to the same index and test performance, e.g.:

CREATE INDEX idx_search_term_trgm
ON public.txs
USING gin (
 contract_call_function_name gin_trgm_ops,
 contract_name gin_trgm_ops
 memo gin_trgm_ops
 ....
);

Open to suggestions ofc.

Copy link
Contributor

@zone117x zone117x Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems fine to me. In the future we could always add another query param that lets users specify a given field(s) to search. And for now it can default to "all".

He1DAr reacted with thumbs up emoji
@He1DAr He1DAr force-pushed the feat/tx-list-function-name-fuzzy-search branch 2 times, most recently from c04af8c to f663d97 Compare October 3, 2024 13:09
@He1DAr He1DAr force-pushed the feat/tx-list-function-name-fuzzy-search branch from f663d97 to 7191ba7 Compare October 3, 2024 13:14
@He1DAr He1DAr marked this pull request as ready for review October 3, 2024 13:14
@He1DAr He1DAr force-pushed the feat/tx-list-function-name-fuzzy-search branch from 7191ba7 to 60c3517 Compare October 10, 2024 20:18
@He1DAr He1DAr requested a review from rafaelcr October 10, 2024 20:26
? sql`AND contract_call_contract_id = ${contractId}`
: sql``;

const pgTrgmInstalled = await this.isPgTrgmInstalled(sql);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be inlined to avoid another round-trip query to postgres? E.g. something like

const searchTermFilterSql = searchTerm
 ? sql`
 AND (
 CASE
 WHEN EXISTS (
 SELECT 1
 FROM pg_extension
 WHERE extname = 'pg_trgm'
 )
 THEN similarity(contract_call_function_name, ${searchTerm}) > ${TRGM_SIMILARITY_THRESHOLD}
 ELSE contract_call_function_name ILIKE '%' || ${searchTerm} || '%'
 END
 )
 `
 : sql``;

He1DAr reacted with thumbs up emoji
@He1DAr He1DAr force-pushed the feat/tx-list-function-name-fuzzy-search branch from 60c3517 to aaebbf7 Compare October 11, 2024 14:46
@He1DAr He1DAr requested a review from zone117x October 11, 2024 15:10
Copy link
Contributor

@zone117x zone117x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me. My last concern before merging is around query performance. @He1DAr can you work with devops to get this branch deployed to our dev environment?

He1DAr reacted with thumbs up emoji
Copy link
Collaborator

@rafaelcr rafaelcr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I agree that we should deploy this to dev to give it a test before merging

He1DAr reacted with thumbs up emoji
@zone117x zone117x linked an issue Mar 25, 2025 that may be closed by this pull request
Copy link
Contributor

saralab commented May 2, 2025

@zone117x @rafaelcr What's the latest on this? We discussed the next steps here a while back, I would like to close all stale PRs ASAP

Copy link
Contributor

zone117x commented May 3, 2025

In the associated issue I outlined some possible next steps that could be taken to make this feature work #2164 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rafaelcr rafaelcr rafaelcr approved these changes

+1 more reviewer

@zone117x zone117x zone117x left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

Status: 👀 In Review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Investigate fuzzy search query performance discrepancy

AltStyle によって変換されたページ (->オリジナル) /