fix(search): order cross-chain results by block time and escape LIKE wildcards - #35
fix(search): order cross-chain results by block time and escape LIKE wildcards#35Ayush7614 wants to merge 2 commits into
Conversation
…wildcards - searchLaunches pre-limit and JS tiebreak used raw block numbers across chains, hiding exact matches on low-height chains; both now use block time (chain id, block number tiebreaks), matching the newestFirst invariant - findLaunchChain picks the newest row by block time, not height - escape %/_ in ILIKE patterns (ESCAPE '\') so they match literally - compareSearchHit + escapeLike are pure and unit-tested
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Comment |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Ordering cross-chain results by block time is a good fix. The wildcard change needs one correction before this can land: JavaScript consumes the backslash in the tagged template, so the SQL receives an empty ESCAPE string and does not treat the helper's backslashes as escapes.
I confirmed this from the exact-head TypeScript template's cooked text and the postgres driver's use of cooked template strings. This is a query-construction reproduction, not a live-database test. The helper tests do not exercise that boundary.
Requesting changes for the escaping issue below; the rest of the search ordering change can stay in this PR.
| // "_" matches those literal characters, not every row. Pre-limit orders by | ||
| // block time — never raw block numbers across chains (Base heights dwarf | ||
| // Robinhood's, so the old ORDER BY hid exact matches on the low chain). | ||
| : await db<Raw[]>`${db.unsafe(SELECT)} WHERE (l.name ILIKE ${"%" + escapeLike(n) + "%"} ESCAPE '\' OR l.symbol ILIKE ${"%" + escapeLike(n) + "%"} ESCAPE '\') ${chainCond} ORDER BY l.block_time DESC, l.chain_id DESC, l.block_number DESC LIMIT 200`; |
There was a problem hiding this comment.
The single backslash in this JavaScript template is consumed before the SQL reaches postgres: both fragments cook to ESCAPE ''. That disables SQL escaping, while escapeLike(n) still inserts backslashes, so literal % and _ searches no longer match as intended. Please double the backslash at the JavaScript layer, or use an unambiguous fixed escape character with a matching helper. Add a regression that inspects the generated SQL or executes literal %, _ and backslash searches, rather than only testing escapeLike.
There was a problem hiding this comment.
@Vasanthdev2004 Thanks — doubled to ESCAPE '\\\\' in source so cooked SQL is now ESCAPE '\\' and escapeLike's backslashes are honored. Verified the cooked SQL no longer collapses to ESCAPE ''.
… '\'
JS template cooked text consumes a single backslash, so ESCAPE '\' in source was cooking to ESCAPE '' (empty escape) and the helper's backslashes were ignored. Using ESCAPE '\\' in source cooks to ESCAPE '\' in SQL, matching escapeLike('\%','\_','\\').
Verified: search.test.ts 9/9; query-construction check shows ESCAPE '\' in cooked SQL.
|
@Vasanthdev2004 Thanks for catching this! Fixed the ESCAPE handling:
Pushed to |
searchLaunches ordered cross-chain results by raw block number (pre-limit) and tie-broke in JS the same way, so exact matches on low-height chains never reached the ranker; findLaunchChain had the same flaw for the legacy /t/ redirect. Both now use block time (chain id, block number tiebreaks), matching the newestFirst invariant the list sorts already follow. LIKE metacharacters (%_) are escaped with ESCAPE backslash so they match literally. New pure compareSearchHit + escapeLike are unit-tested (search.test.ts: 9 pass). Verified: full app suite 337/337, lint, typecheck, build.