Skip to content

Conversation

@samrose
Copy link
Collaborator

@samrose samrose commented Oct 21, 2025

Changes from INDATA-152 to INDATA-152-run-server

Overview

This document analyzes only the changes made in the INDATA-152-run-server branch compared to the INDATA-152 branch. These changes focus on aligning the test infrastructure more closely with production Ansible configuration patterns and improving transaction handling.

Changes: 66 files changed, 1,388 insertions(+), 17,222 deletions(-)

The massive reduction in lines (-15,834 net) is primarily due to changes in expected output files where tests now create and rollback extensions instead of showing permanent installations.


Core Philosophy Change

Before (INDATA-152)

  • Tests ran on a pre-configured database with all extensions installed via prime.sql
  • Extensions persisted across tests
  • Tests modified global database state

After (INDATA-152-run-server)

  • Tests run within BEGIN/ROLLBACK transactions
  • Each test creates only the extensions it needs
  • Database state is automatically cleaned up after each test
  • Only production default extensions are pre-installed via prime-production.sql

Why: This matches how production users work—they start with default extensions and create additional ones as needed within their own transactions.


1. Configuration Directory Handling (run-server.sh.in)

Before

SUPAUTILS_CONFIG_FILE=@SUPAUTILS_CONF_FILE@
LOGGING_CONFIG_FILE=@LOGGING_CONF_FILE@
READREPL_CONFIG_FILE=@READREPL_CONF_FILE@

mkdir -p "$DATDIR/conf.d"
cp "$READREPL_CONFIG_FILE" "$DATDIR/conf.d/read-replica.conf"
sed "s|...|" "$SUPAUTILS_CONFIG_FILE" > "$DATDIR/conf.d/supautils.conf"

After

# Copy entire conf.d directory from postgresql_config
POSTGRESQL_CONFIG_DIR="@POSTGRESQL_CONFIG_DIR@"
cp -r "$POSTGRESQL_CONFIG_DIR/conf.d" "$DATDIR/"

# Make conf.d files writable (they're read-only from Nix store)
chmod -R u+w "$DATDIR/conf.d"

Why:

  • Simpler: Copy entire directory instead of individual files
  • Matches Ansible: Production uses directory-based deployment
  • Fewer variables: Single POSTGRESQL_CONFIG_DIR instead of multiple file paths
  • Fixes Nix permissions: Files from Nix store are read-only, chmod makes them writable for sed operations

2. Reserved Roles Toggle During Migrations

New Feature

# BEFORE migrations: Comment out reserved_roles
sed -i.bak \
    -e "s|supautils.extension_custom_scripts_path = '...'|...|" \
    -e "s|^supautils.reserved_roles|#supautils.reserved_roles|" \
    "$DATDIR/conf.d/supautils.conf"

# ... run migrations ...

# AFTER migrations: Restore reserved_roles
sed -i.bak "s|^#supautils.reserved_roles|supautils.reserved_roles|" \
    "$DATDIR/conf.d/supautils.conf"

Why:

  • Migrations need to create system roles (postgres_fdw_handler, etc.)
  • supautils.reserved_roles blocks role creation for security
  • Toggle allows migrations to run, then re-enables security for normal operations

3. After-Create Script Execution

New Feature

# Run after-create scripts for extensions not managed by privileged_extensions
echo "Running after-create scripts for non-privileged extensions"
if psql ... -c "SELECT 1 FROM pg_extension WHERE extname = 'pgmq'" | grep -q 1; then
    echo "Running pgmq after-create script"
    psql ... -f "$DATDIR/extension-custom-scripts/pgmq/after-create.sql"
fi

Why:

  • pgmq is not in supautils privileged_extensions list
  • Supautils doesn't automatically run its after-create script
  • Manual execution ensures pgmq functions have correct ownership (as configured in Ansible)
  • Matches production: Same after-create scripts run in both test and production environments

4. Production Priming (prime-production.sql)

New File (replaces prime.sql)

-- Production default extensions (enabled on every new Supabase project)
create extension if not exists "uuid-ossp" with schema extensions;
create extension if not exists pgcrypto with schema extensions;
create extension if not exists pg_graphql with schema graphql;
create extension if not exists pg_stat_statements with schema extensions;

-- Additional extensions for security vetting
create extension if not exists dblink with schema extensions;
create extension if not exists pgaudit with schema extensions;
create extension if not exists postgis with schema extensions;
create extension if not exists pg_repack with schema extensions;
create extension if not exists pgsodium;

Why:

  • Minimal: Only 9 default extensions (vs. ~80 in old prime.sql)
  • Production-accurate: Matches what customers see on project creation
  • Security focus: Includes extensions with SECURITY DEFINER functions that need validation
  • Test independence: Other extensions created on-demand by individual tests

5. Transaction-Wrapped Tests

Before (INDATA-152)

-- http.sql
SELECT status FROM http_get('...');
-- ... more tests

After (INDATA-152-run-server)

BEGIN;

set client_min_messages = warning;
create extension if not exists http with schema extensions;

-- Test basic HTTP GET request
SELECT status FROM http_get('...');
-- ... more tests

ROLLBACK;

Applied to: All 40+ test files

Why:

  • Isolation: Each test starts with clean state
  • No side effects: ROLLBACK cleans up all changes
  • Matches production pattern: Users create extensions within their own transactions
  • Easier debugging: Clear boundaries between tests
  • Faster tests: No need to drop/recreate extensions between tests

6. Savepoint Error Handling

pgmq.sql

Before

select pgmq.create('F--oo');  -- ERROR: queue name contains invalid characters
select pgmq.create('F$oo');   -- ERROR: current transaction is aborted
select pgmq.create($$F'oo$$); -- ERROR: current transaction is aborted

After

SAVEPOINT test_invalid_names;
select pgmq.create('F--oo');
ROLLBACK TO SAVEPOINT test_invalid_names;

SAVEPOINT test_invalid_names;
select pgmq.create('F$oo');
ROLLBACK TO SAVEPOINT test_invalid_names;

SAVEPOINT test_invalid_names;
select pgmq.create($$F'oo$$);
ROLLBACK TO SAVEPOINT test_invalid_names;

Why:

  • Correct error testing: Each error case tested independently
  • No transaction abort: SAVEPOINT prevents entire transaction from aborting
  • Matches PostgreSQL best practice: Use savepoints for expected errors

7. Extension Schema Enforcement

Non-Relocatable Extensions Fixed

pg_tle

-- Before: Tried to create in extensions schema (fails)
create extension if not exists pg_tle with schema extensions;

-- After: Create in required pgtle schema
create schema if not exists pgtle;
create extension if not exists pg_tle with schema pgtle;

pg_surgery

-- Before: No schema specified (fails in tests)
create extension if not exists pg_surgery;

-- After: Explicit schema required
create extension if not exists pg_surgery with schema pg_catalog;

Why:

  • Non-relocatable extensions must be in specific schemas
  • pg_tlepgtle schema (enforced by extension itself)
  • pg_surgerypg_catalog schema (standard location)
  • Catches production errors: Tests now validate schema placement that production enforces

8. Large Test Refactoring (ext_interface tests)

Before (INDATA-152)

-- No transaction, extensions already installed
/* List all extensions... */
SELECT ...

After (INDATA-152-run-server)

BEGIN;

set client_min_messages = warning;

-- Create required schemas
create schema if not exists extensions;
create schema if not exists topology;

-- Create ALL extensions used in production
create extension if not exists address_standardizer with schema extensions;
create extension if not exists bloom with schema extensions;
-- ... 70+ more extensions ...
create extension if not exists pg_tle;
create extension if not exists pgsodium;

/* List all extensions not enabled */
SELECT ...

ROLLBACK;

Applied to:

  • z_15_ext_interface.sql
  • z_17_ext_interface.sql
  • z_orioledb-17_ext_interface.sql

Why:

  • Comprehensive test: Validates ALL extensions can be created
  • Schema validation: Tests correct schema placement for each extension
  • Production simulation: Creates extensions exactly as users would
  • Automatic cleanup: ROLLBACK removes all 70+ extensions after test
  • Output reduction: Expected output files shrunk by ~6000 lines each (extensions no longer permanently installed)

9. Build System Simplification

lib.nix

Before

substitutions = {
  SUPAUTILS_CONF_FILE = "${paths.supaconfFile}";
  LOGGING_CONF_FILE = "${paths.loggingConfFile}";
  READREPL_CONF_FILE = "${paths.readreplConfFile}";
  # ... individual file paths ...
};

# Build phase: copy individual files
mkdir -p $out/etc/postgresql-custom $out/etc/postgresql
cp -r ${paths.configConfDir} $out/etc/postgresql-custom/
cp ${paths.pgconfigFile} $out/etc/postgresql/postgresql.conf
cp ${paths.pgHbaConfigFile} $out/etc/postgresql/pg_hba.conf
# ... many more individual copies ...
chmod 644 $out/etc/postgresql-custom/conf.d/supautils.conf

After

postgresqlConfigBaseDir = builtins.path {
  name = "postgresql_config";
  path = ../../ansible/files/postgresql_config;
};

substitutions = {
  POSTGRESQL_CONFIG_DIR = "${postgresqlConfigBaseDir}";
  # ... other substitutions ...
};

# Build phase: minimal setup
mkdir -p $out/bin
substitute ${../tools/run-server.sh.in} $out/bin/start-postgres-server

Why:

  • Single source of truth: Point to Ansible config directory
  • Fewer variables: One directory path instead of multiple file paths
  • Less brittle: No need to list every config file individually
  • Matches Ansible approach: Directory-based configuration

10. Test Harness Changes

checks.nix

Before

if ! psql ... -Xf ${./tests/prime.sql}; then
  echo "Error executing SQL file"
  exit 1
fi

After

if ! psql ... -Xf ${./tests/prime-production.sql}; then
  echo "Error executing SQL file"
  exit 1
fi

Why:

  • Use new prime-production.sql with minimal production defaults
  • Tests create their own extensions as needed
  • Faster test startup (9 extensions vs. 80+)

11. Migration Test Fixes

migrations/tests/extensions/01-postgis.sql

Before

begin;
do $_$
begin
  if not exists (select 1 from pg_extension where extname = 'orioledb') then
    create extension if not exists postgis_tiger_geocoder cascade;

After

begin;
do $_$
begin
  if not exists (select 1 from pg_extension where extname = 'orioledb') then
    -- create address_standardizer and dependencies first
    create extension if not exists address_standardizer with schema extensions;
    create extension if not exists address_standardizer_data_us with schema extensions;

    create extension if not exists postgis_tiger_geocoder cascade;

Why:

  • postgis_tiger_geocoder depends on address_standardizer
  • Previously relied on prime.sql having these installed
  • Now explicitly creates dependencies with correct schemas
  • Tests migration in isolation: Doesn't assume pre-existing extensions

Summary of Benefits

Production Alignment

✅ Tests use same conf.d structure as Ansible
✅ Tests create extensions same way as users
✅ Tests validate schema enforcement
✅ Tests run after-create scripts same as production

Test Quality

✅ Transaction isolation prevents test interference
✅ Savepoint handling allows proper error testing
✅ No "transaction is aborted" errors
✅ Each test is self-contained and independent

Maintainability

✅ Simpler build system (directory copy vs. individual files)
✅ Fewer configuration variables
✅ Clear test boundaries (BEGIN/ROLLBACK)
✅ Expected output files are smaller (no permanent extension installations)

Development Experience

✅ Faster test startup (9 default extensions vs. 80+)
✅ Easier to understand test failures
✅ Tests catch production issues earlier
✅ Configuration changes automatically picked up (directory-based)


File Change Breakdown

Category Files Changed Purpose
Test SQL files 40+ files Add BEGIN/ROLLBACK, create extensions on-demand
Expected output files 40+ files Update for transaction-wrapped behavior
Build system 2 files (lib.nix, checks.nix) Simplify to directory-based config
Run server script 1 file conf.d copying, reserved_roles toggle, after-create scripts
New files 1 file (prime-production.sql) Production defaults only
Migration tests 1 file Fix address_standardizer dependency

Total: 66 files, net reduction of ~15,834 lines (primarily in expected output files)


Migration Guide

For Test Writers

Old pattern:

-- Test assumed extension already existed
SELECT * FROM extension_function();

New pattern:

BEGIN;

set client_min_messages = warning;
create extension if not exists my_extension with schema extensions;

-- Now test the extension
SELECT * FROM extension_function();

ROLLBACK;

For Non-Relocatable Extensions

Always specify the schema:

create extension if not exists pg_tle with schema pgtle;
create extension if not exists pg_surgery with schema pg_catalog;
create extension if not exists pgsodium; -- uses default 'pgsodium' schema

For Error Testing

Use savepoints:

SAVEPOINT test_error;
-- operation that should fail
SELECT should_error();
ROLLBACK TO SAVEPOINT test_error;

Conclusion

The changes from INDATA-152 to INDATA-152-run-server represent a fundamental shift in test philosophy: tests now simulate the production user experience rather than running against a pre-configured database. This improves test accuracy, catches production issues earlier, and makes the test suite more maintainable while reducing overall code size.

@samrose samrose marked this pull request as ready for review October 22, 2025 01:50
@samrose samrose requested review from a team as code owners October 22, 2025 01:50
and be more accurate with production conditions
@samrose samrose force-pushed the INDATA-152-run-server branch from 4ad3297 to 930d9f3 Compare October 22, 2025 09:48
@samrose samrose requested a review from hunleyd October 22, 2025 15:40
@hunleyd
Copy link
Contributor

hunleyd commented Oct 22, 2025

awesome work @samrose ! ready to merge if you're happy.

@samrose samrose merged commit 1f82f5b into INDATA-152 Oct 22, 2025
24 of 25 checks passed
@samrose samrose deleted the INDATA-152-run-server branch October 22, 2025 20:57
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.

3 participants