feat(postgres-estimated-rows): pg Estimated Rows on Data Warehouse Sync #27634

phixMe · 2025-01-17T00:20:04Z

Problem

We did not provide row estimates for Postgres so users aren't really able to calculate cost effectively

Changes

Adds a call to reference a table level estimates for row counts
Returns and renders before syncing sources

Does this work well for both Cloud and self-hosted?

Yes

How did you test this code?

Used a real postgres instance and got some row estimates.

github-actions · 2025-01-17T00:54:52Z

Size Change: +33 B (0%)

Total Size: 1.16 MB

ℹ️ View Unchanged

Filename	Size	Change
`frontend/dist/toolbar.js`	1.16 MB	+33 B (0%)

_{compressed-size-action}

posthog-bot · 2025-01-17T01:16:50Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2025-01-17T01:36:09Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

phixMe · 2025-01-20T17:58:30Z

@Gilbert09 I tried this out on metabase and saw that every 'estimate' returned zero, so I assume that this isn't going to work in our/all environments, I'm going to switch to a more traditional generated count(*) type query... Unfortunately, I cannot join literals and use them with table names in a subquery, so I'll have to generate a second union style query to accomplish this... On the plus side it would no longer be an estimated value...

Gilbert09

A couple of questions before approving, but this looks good

Gilbert09 · 2025-01-21T16:49:54Z

frontend/src/lib/lemon-ui/LemonTable/types.ts

@@ -20,6 +20,8 @@ export interface LemonTableColumn<T extends Record<string, any>, D extends keyof
    /** Tooltip to display on title hover. An info icon ("i" in circle) is shown when a tooltip is available. */
    tooltip?: string
    key?: string
+    /** If true, the column is not displayed. Optional, defaults to not disabled. */
+    is_disabled?: boolean


maybe is_visible is better suited here - we use disabled elsewhere for when a UI element is still visible but not interactive

That makes sense, I do want to keep it as a negative for semantic and optional parameterization reasons, so I'll switch to isHidden.

Gilbert09 · 2025-01-21T16:52:14Z

posthog/warehouse/models/external_data_schema.py

+            if not tables:
+                return {}
+            union = [
+                f"SELECT '{table[0]}' AS table_name, COUNT(*) AS row_count FROM {schema}.{table[0]}" for table in tables


Is it possible to SQL inject here? Can we parameterize this query instead? https://www.psycopg.org/psycopg3/docs/basic/params.html

I considered this, the injection would need to be in the table name which I don't think is possible unless there is some crazy escape sequence hack. I'd also like to be more confident, so I'm going to add some substitutions when creating the query expression. I think the substituted approach reads a bit better as well, so no complaints there either.

Gilbert09 · 2025-01-21T16:53:06Z

posthog/warehouse/models/external_data_schema.py

@@ -309,6 +309,45 @@ def filter_postgres_incremental_fields(columns: list[tuple[str, str]]) -> list[t
    return results


+def get_postgres_row_count(


This is all fine to live here right now, but we need to figure out a more longer-term solution for these SQL sources - they're all kinda over the place right now - some future pipeline work

Yeah, I noticed it's a little chaotic with all of these definitions. Certainly worth consolidating some base interfaces for the corresponding providers.

…nto feat(postgres-estimated-rows)

posthog-bot · 2025-01-21T18:28:13Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Gilbert09

This looks great!

…nto feat(postgres-estimated-rows) # Conflicts: # frontend/__snapshots__/replay-player-success--second-recording-in-list--dark.png

posthog-bot · 2025-01-22T17:07:39Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2025-01-22T17:28:39Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

* master: (103 commits) feat(postgres-estimated-rows): pg Estimated Rows on Data Warehouse Sync (#27634) fix: revert darkmode class toggle, updated content on fills (#27783) chore: upgrade posthog-js (#27790) chore(editor-3001): add back join actions (#27740) feat: Add person distinct ID overrides squash job (as dagster job) (#27710) fix(created-by-sources): Adding `created_by` to sources (#27751) Revert "feat(data-warehouse): V2 pipeline release " (#27791) fix: typo for feature flags (#27786) fix(defer-unmounting): Defer unmounting of react elements (#27742) feat(data-warehouse): V2 pipeline release (#27732) fix(data-warehouse): Ensure dates are actual datetime formats (#27777) fix: enable hot reload for the products dir (#27746) fix: assignee selector when null (#27737) chore: clarify rrweb imports (#27776) chore(deps): Update posthog-js to 1.207.3 (#27779) feat(retention): filters on start/return event (#27770) fix(experiments): only show supported math functions (#27589) feat(web-analytics): Set unique conversions graph when adding conversions goal (#27774) chore: color design system part 1: banner and accents (#27756) chore(experiments): Add tests for funnel attribution options (#27752) ...

sentry-io · 2025-01-23T08:27:59Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ InsufficientPrivilege: permission denied for table email_meta_tag /api/projects/{parent_lookup_team_id}/external_... View Issue
‼️ OperationalError: connection to server at "development-databasewriter2462cc03-jonothxrzxcf.czu26m4ieacl.us-west-2.r... /api/projects/{parent_lookup_team_id}/external_... View Issue

_{Did you find this useful? React with a 👍 or 👎}

…nc (#27634) Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

phixMe added 3 commits January 16, 2025 16:12

Add estimated rows for postgres db engine.

82708d9

Handling 0 falsey evaluation.

ecaf6ce

Adding None default case

a46f5c5

Update UI snapshots for chromium (1)

4d74c2d

Update UI snapshots for chromium (1)

25513fe

phixMe and others added 5 commits January 20, 2025 11:42

Updating query and adding is_disabled column feature.

cda94d9

Merge branch 'master' into feat(postgres-estimated-rows)

481def5

Update query snapshots

e51025f

Fixing test suite with mocks.

20ee8a2

Merge branch 'master' into feat(postgres-estimated-rows)

f9410be

phixMe requested review from Gilbert09 and EDsCODE January 21, 2025 16:42

phixMe marked this pull request as ready for review January 21, 2025 16:43

Gilbert09 reviewed Jan 21, 2025

View reviewed changes

phixMe and others added 4 commits January 21, 2025 09:50

Code review feedback

bd67b3c

Merge remote-tracking branch 'origin/feat(postgres-estimated-rows)' i…

4ae2aa8

…nto feat(postgres-estimated-rows)

Merge branch 'master' into feat(postgres-estimated-rows)

723a714

Update UI snapshots for chromium (1)

042c59f

Gilbert09 approved these changes Jan 22, 2025

View reviewed changes

EDsCODE approved these changes Jan 22, 2025

View reviewed changes

phixMe and others added 4 commits January 22, 2025 08:47

Merge branch 'refs/heads/master' into feat(postgres-estimated-rows)

cd52651

Merge remote-tracking branch 'origin/feat(postgres-estimated-rows)' i…

133ef49

…nto feat(postgres-estimated-rows) # Conflicts: # frontend/__snapshots__/replay-player-success--second-recording-in-list--dark.png

Merge branch 'master' into feat(postgres-estimated-rows)

437d365

Update UI snapshots for chromium (2)

a768da1

Update UI snapshots for chromium (2)

4dfc8b5

phixMe merged commit f5b9e89 into master Jan 22, 2025
99 checks passed

phixMe deleted the feat(postgres-estimated-rows) branch January 22, 2025 18:16

timgl pushed a commit that referenced this pull request Jan 28, 2025

feat(postgres-estimated-rows): pg Estimated Rows on Data Warehouse Sy…

caaffbb

…nc (#27634) Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(postgres-estimated-rows): pg Estimated Rows on Data Warehouse Sync #27634

feat(postgres-estimated-rows): pg Estimated Rows on Data Warehouse Sync #27634

phixMe commented Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2025 •

edited

Loading

posthog-bot commented Jan 17, 2025

posthog-bot commented Jan 17, 2025

phixMe commented Jan 20, 2025

Gilbert09 left a comment

Gilbert09 Jan 21, 2025

phixMe Jan 21, 2025

Gilbert09 Jan 21, 2025

phixMe Jan 21, 2025

Gilbert09 Jan 21, 2025

phixMe Jan 21, 2025

posthog-bot commented Jan 21, 2025

Gilbert09 left a comment

posthog-bot commented Jan 22, 2025

posthog-bot commented Jan 22, 2025

sentry-io bot commented Jan 23, 2025 •

edited

Loading

		@@ -309,6 +309,45 @@ def filter_postgres_incremental_fields(columns: list[tuple[str, str]]) -> list[t
		return results


		def get_postgres_row_count(

feat(postgres-estimated-rows): pg Estimated Rows on Data Warehouse Sync #27634

feat(postgres-estimated-rows): pg Estimated Rows on Data Warehouse Sync #27634

Conversation

phixMe commented Jan 17, 2025 • edited Loading

Problem

Changes

Does this work well for both Cloud and self-hosted?

How did you test this code?

github-actions bot commented Jan 17, 2025 • edited Loading

posthog-bot commented Jan 17, 2025

📸 UI snapshots have been updated

posthog-bot commented Jan 17, 2025

📸 UI snapshots have been updated

phixMe commented Jan 20, 2025

Gilbert09 left a comment

Choose a reason for hiding this comment

Gilbert09 Jan 21, 2025

Choose a reason for hiding this comment

phixMe Jan 21, 2025

Choose a reason for hiding this comment

Gilbert09 Jan 21, 2025

Choose a reason for hiding this comment

phixMe Jan 21, 2025

Choose a reason for hiding this comment

Gilbert09 Jan 21, 2025

Choose a reason for hiding this comment

phixMe Jan 21, 2025

Choose a reason for hiding this comment

posthog-bot commented Jan 21, 2025

📸 UI snapshots have been updated

Gilbert09 left a comment

Choose a reason for hiding this comment

posthog-bot commented Jan 22, 2025

📸 UI snapshots have been updated

posthog-bot commented Jan 22, 2025

📸 UI snapshots have been updated

sentry-io bot commented Jan 23, 2025 • edited Loading

Suspect Issues

phixMe commented Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2025 •

edited

Loading

sentry-io bot commented Jan 23, 2025 •

edited

Loading