Skip to content

feat: set up some Postgres experiments to be used for API testing - #6150

Merged
michaelkedar merged 8 commits into
google:masterfrom
michaelkedar:🧪💽
Oct 16, 2026

Hidden character warning

The head ref may contain hidden characters: "\ud83e\uddea\ud83d\udcbd"
Merged

michaelkedar merged 8 commits into
google:masterfrom
michaelkedar:🧪💽

Conversation

@michaelkedar

Copy link
Copy Markdown
Member

Setting up an experimental PostgreSQL database so we can test the performance of Postgres for our API queries.
I've put a bunch of things in temporary directories because they're not the final design and I'm planning on deleting this stuff when I start implementing the actual PostgreSQL database.

This PR includes

  • Temporary terraform files to set up the Cloud SQL Postgres database
  • Goose migration files to set up some basic incomplete tables that can be used to simulate API querying
    • I've already populated these on the test instance from an all.zip export and the AffectedCommits backup.
    • Planning on just copying the test instance db over to prod to do live testing there
  • A small sqlc setup with a few queries to be used for API dual read testing

In an eventful follow-up, I'll make a dual-read VulnerabilityStore interface so we can benchmark the performance of this Postgres design vs Datastore.

@michaelkedar
michaelkedar requested review from a team and another-rex and removed request for another-rex October 13, 2026 03:27
return ""
}

func normalizeRepo(repoURL string) string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we normalize in gitter too? maybe we should make this a util to be shared between packages.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This normalization is for the API side to do string matching of GIT tags.
Maybe in a follow up

Comment thread db_test/20261001064316_test.sql
data JSONB NOT NULL,
proto BYTEA NOT NULL
);
CREATE TABLE affected_commits (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need references for vulnerabilities here as well? just like vuln_commits and affected_versions?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't for a bit of an optimisation reason:
If we were to add REFERENCES vulnerabilities(id) here, we'd either have to do a full table scan or create a new vuln_id index on the table (which we already have vuln_commits for to do the ID -> commits lookup more efficiently, which we can use for deletes as well)

I've added a comment here to explain also

Comment thread go/internal/database/postgres/affected_versions.go Outdated
}
}

func (s *VulnerabilityStore) versionIsMatch(version, pkgName, rowEco string, versions []string, eventsJSON []byte) bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

buildVersionedWithEcoMatcher skips range-based matching for Alpine and Ubuntu but versionIsMatch does not. Is this intentional?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think it was (the same behaviour is in the Datastore implementation) but...
versionIsMatch is only really called when someone queries with no ecosystem specified, which is a strange use-case and might as well return the most lenient matches possible.

@michaelkedar
michaelkedar merged commit a7ff5a4 into google:master Oct 16, 2026
23 checks passed
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.

2 participants