Skip to content

Let an app opt out of row estimate hints - #2921

Merged
stopachka merged 1 commit into
mainfrom
disable-row-hints-apps
Sep 17, 2026
Merged

stopachka merged 1 commit into
mainfrom
disable-row-hints-apps

Conversation

@stopachka

Copy link
Copy Markdown
Contributor

Row estimate hints (#2755) tell Postgres how many rows a join returns. For some apps they flip the next join the wrong way around. Postgres estimates the unparameterized scan of an attribute at 1 row, so once a hint says the driving CTE has 20 rows, it scans every triple of the attribute and filters each one against the CTE, instead of doing 20 index lookups.

One prod plan (app 6f4b4047, hinted Rows(... #20)):

Nested Loop  (actual rows=2179)
  Join Filter: (m_18.m_18_entity_id = t19.entity_id)
  Rows Removed by Join Filter: 25179901
  -> Index Scan using ea_index on triples t19  (plan rows=1, actual rows=286160)
  -> CTE Scan on m_18  (actual rows=88 loops=286160)

I ran 97 of the top queries by database time through instaql/explain in prod, with and without row hints:

app query with row hints without
36dce386 awards, 5 levels deep 527 ms under 1 ms
6f4b4047 program weeks, $in of 4 9,389 ms 20 ms
6f4b4047 program days, nested 624 ms 6 ms
d6a4ea80 conversationParticipants 1,177 ms 50 ms
d6a4ea80 112207219 1,712 ms 12 ms
7e356cba tasks with messages 293 ms 29 ms
bd26fd12 436384447 2,057 ms 43 ms

22 queries were at least 1.5x faster without row hints, 2 were slower (130 ms to 204 ms, and 14 ms to 21 ms), and the rest did not change.

disable-row-hints-query-hashes already fixes a single query, and the verified hashes are listed there now. It does not scale for apps like 6f4b4047, where nearly every query is affected and the hash changes with the length of each $in list. This adds disable-row-hints-apps, a list of app ids that skip row hints. Everything else about their hints stays the same, and other apps are untouched.

["6f4b4047-f757-4226-846e-e28f6674abca", "d6a4ea80-4b9f-43c6-8013-6bbeadc78da4"]

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c1dac6d8-07f8-4d29-ae38-17eedec564ac

📥 Commits

Reviewing files that changed from the base of the PR and between bd61f3e and c5fbd09.

📒 Files selected for processing (3)
  • server/src/instant/db/datalog.clj
  • server/src/instant/flags.clj
  • server/test/instant/db/row_hints_flag_test.clj

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change adds the :disable-row-hints-apps flag. Row hint generation now checks the current application ID. Tests verify disabled and unaffected applications.

Changes

Per-app row hint control

Layer / File(s) Summary
Disable-app flag parsing
server/src/instant/flags.clj
The flags transformation parses :disable-row-hints-apps values into UUID sets.
Application-aware row hint gating
server/src/instant/db/datalog.clj, server/test/instant/db/row_hints_flag_test.clj
enable-row-hints? accepts the application ID and disables hints when that ID is configured. The test verifies that configured applications lose row hints while other applications retain them.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: dwwoelfel

Merge Risk: ⚪ Minimal · up to c5fbd

The per-app row-hint control is wired through compatible UUID flag parsing and the updated caller; no actionable merge risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: allowing apps to opt out of row estimate hints.
Description check ✅ Passed The description directly explains the row hint performance issue, the new app-level configuration, the affected apps, and the expected behavior for other apps.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@stopachka
stopachka merged commit 70ff138 into main Sep 17, 2026
34 checks passed
@stopachka
stopachka deleted the disable-row-hints-apps branch September 17, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant