Skip to content

Commit

Permalink
feat (client): support for PG on the client (#1038)
Browse files Browse the repository at this point in the history
This PR adds support for Postgres on the client:
- adds DB drivers for Postgres in NodeJS and in Tauri
- modifies the initial Electric schema (meta tables)
- modifies the queries executed by the `BundleMigrator`
- modifies the triggers to also support PG
- modifies the Satellite process to also support PG
- modifies the CLI to bundle both SQLite and PG migrations
- modifies the DAL to transform JS values to PG when writing and the
other way around when reading

**To test** this PR you will need to:
- use a local build of Electric from `alco/vax-1659-postgresql-dialect`
- npm pack the generator
- use the packed generator in the ts-client
- npm pack the ts-client
- use the packed version of ts-client in the example app (and use
node-postgres or tauri-postgres driver)

Limitations:
- Floating-point numbers that are out of range cannot be inserted (PG
throws an error, in contrast to SQLite which silently converts them to
+Inf/-Inf (for too big values) or 0 (for too small values))
- Not sure if PGlite version currently supports infinity values and/or
`NaN` for floating-point numbers. Our triggers do some casting of these
values, need to test this with some e2e tests or actual tauri
application to see if it works or not.

A couple of things to note regarding the implementation:
- Had to change a trigger in both SQLite and PG for storing the JSON of
a row's PK:
- In PG, `jsonb` adds whitespace and re-orders keys. Whereas, SQLite has
no whitespace and preserves the ordering of keys. So in PG we use `json`
instead of `jsonb` and strip any whitespace but that has the side effect
of also removing `null` values. Hence, we also remove `null` values in
SQLite.
- Postgres requires migrations coming from Electric to define all FKs as
`DEFERRABLE`
- We no longer defer foreign key checks before applying an incoming
transaction as PG does not like deferred constraints when trying to
apply some DDL. This happens because PG wants to run all trigger events
before applying DDL but they are deferred so they are only allowed to
run when committing so PG is stuck. So if a TX contains DML and DDL then
when it encounters the DDL we get errors like this: `cannot ALTER TABLE
"parent" because it has pending trigger events`

Remains to be done (perhaps in a follow-up PR):
- e2e tests for PG client

------------

For reviewers:

- The crux of this PR consists of modifying the TS-client to abstract
away dialect-specific SQL queries behind a query builder abstraction:
- `src/migrators/query-builder/builder.ts` defines an abstract
`QueryBuilder` class that every dialect must implement
- `src/migrators/query-builder/sqliteBuilder.ts` and
``src/migrators/query-builder/pgBuilder.ts` implement the abstract class
for SQLite and Postgres respectively
- When electrifying a database, the dialect can (optionally) be provided
in the config object. It defaults to `'SQLite'`. Postgres drivers'
`electrify` function pass `'Postgres'` dialect through this config
object. The DAL checks the dialect in order to apply the right
conversions from JS values to SQLite/PG values and back.
- The main changes are in `src/migrators/bundle.ts`,
`src/migrators/schema.ts`, and `src/migrators/triggers.ts` to use the
dialect-specific query builder object.
- Likewise, the satellite process (`src/satellite/process.ts`) has been
changed to use the dialect-specific query builder for constructing its
specialised queries.
- Another important change consists of supporting data types such as
booleans, dates, json, etc.
- In SQLite such values were converted to strings and stored as strings
(since SQLite has no real support for them).
- Postgres has support for those data types so we want to leverage that.
- Again, we abstracted away the data type conversions behind a
`Converter` interface (`src/client/conversions/converter.ts`).
- Then we created 2 specific converters, 1 for SQLite
`src/client/conversions/sqlite.ts` and 1 for PG
`src/client/conversions/postgres.ts`
- Similar to the value conversions, we also had to change
serialisation/deserialisation of values in the satellite client.
Currently it was encoding SQLite values on the wire and decoding values
from the wire into SQLite values. We abstracted those encoders/decoders
and created 2 instances of each, one for SQLite and one for PG (cf.
`src/util/encoders`).

---------

Co-authored-by: Oleksii Sholik <oleksii@sholik.dev>
Co-authored-by: msfstef <stefanos@electric-sql.com>
Co-authored-by: Sam Willis <sam.willis@gmail.com>
Co-authored-by: msfstef <msfstef@gmail.com>
  • Loading branch information
5 people committed May 2, 2024
1 parent c8e6981 commit 450a65b
Show file tree
Hide file tree
Showing 220 changed files with 23,124 additions and 8,138 deletions.
7 changes: 7 additions & 0 deletions .changeset/breezy-plums-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@core/electric": patch
"electric-sql": patch
"@electric-sql/prisma-generator": patch
---

Support for a local Postgres database on the client. Also introduces drivers for node Postgres and PGlite.
2 changes: 1 addition & 1 deletion .github/workflows/clients_typescript_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ jobs:
- name: Build
run: make build
- name: Run tests
run: make tests
run: make tests-CI
maybe_publish:
runs-on: ubuntu-latest
needs: [test, check_types, verify_formatting]
Expand Down
94 changes: 92 additions & 2 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ on:
# Root files
- "*"
- "!pnpm-lock.yaml"
- "!clients/typescript/**" # Satellite tests run in a separate workflow
# CI files not related to GH actions
- ".buildkite/**"
- "**/README.md"
Expand Down Expand Up @@ -80,7 +81,7 @@ jobs:

- run: make lux
- run: make deps pull
- run: make test_only
- run: make test-no-satellite
id: tests
env:
ELECTRIC_IMAGE_NAME: electric-sql-ci/electric
Expand All @@ -92,7 +93,7 @@ jobs:
if: ${{ failure() && steps.tests.outcome == 'failure' }}
with:
name: lux_logs
path: e2e/lux_logs/run_*
path: e2e/**/lux_logs/run_*
- name: Upload test results to Buildkite analytics
if: ${{ !cancelled() && steps.tests.outcome != 'skipped' && env.BUILDKITE_ANALYTICS_TOKEN != '' }}
working-directory: e2e/lux_logs/latest_run
Expand All @@ -110,3 +111,92 @@ jobs:
-F "run_env[commit_sha]=$GITHUB_SHA" \
-F "run_env[url]=https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" \
https://analytics-api.buildkite.com/v1/uploads
e2e_satellite_tests:
name: E2E Satellite tests
runs-on: electric-e2e-8-32
strategy:
matrix:
dialect: [SQLite, Postgres]
defaults:
run:
working-directory: e2e
env:
BUILDKITE_ANALYTICS_TOKEN: ${{ secrets.BUILDKITE_TEST_ANALYTICS_E2E }}
DIALECT: ${{ matrix.dialect }}
steps:
- uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 0

- name: Inject slug/short variables
uses: rlespinasse/github-slug-action@v4
- name: Inject variables for `docker buildx` github actions caching
uses: crazy-max/ghaction-github-runtime@v2
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2
- name: Log in to the Container registry
uses: docker/login-action@f054a8b539a109f9f41c372932f1ae047eff08c9
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}
- uses: erlef/setup-beam@v1
with:
otp-version: ${{ env.OTP_VERSION }}
elixir-version: ${{ env.ELIXIR_VERSION }}

- run: |
echo "ELECTRIC_VERSION=$(make --silent print_version_from_git)" >> $GITHUB_ENV
working-directory: components/electric
- run: make docker-build-ci
env:
ELECTRIC_IMAGE_NAME: electric-sql-ci/electric
working-directory: components/electric
- run: make docker-build-ws-client
env:
ELECTRIC_CLIENT_IMAGE_NAME: electric-sql-ci/electric-ws-client
working-directory: components/electric

- name: Cache built lux
uses: actions/cache@v3
with:
path: |
e2e/lux/bin
e2e/lux/ebin
e2e/lux/priv
key: ${{ runner.os }}-luxbuilt-${{ env.OTP_VERSION }}-${{ env.ELIXIR_VERSION }}

- run: make lux
- run: make deps pull
- run: make test-satellite-only
id: tests
env:
ELECTRIC_IMAGE_NAME: electric-sql-ci/electric
ELECTRIC_CLIENT_IMAGE_NAME: electric-sql-ci/electric-ws-client
ELECTRIC_IMAGE_TAG: ${{ env.ELECTRIC_VERSION }}

- name: Upload lux logs
uses: actions/upload-artifact@v3
if: ${{ failure() && steps.tests.outcome == 'failure' }}
with:
name: lux_logs
path: e2e/**/lux_logs/run_*
- name: Upload test results to Buildkite analytics
if: ${{ !cancelled() && steps.tests.outcome != 'skipped' && env.BUILDKITE_ANALYTICS_TOKEN != '' }}
working-directory: e2e/lux_logs/latest_run
run: |
curl \
-X POST \
--fail-with-body \
-H "Authorization: Token token=\"$BUILDKITE_ANALYTICS_TOKEN\"" \
-F "data=@lux_junit.xml" \
-F "format=junit" \
-F "run_env[CI]=github_actions" \
-F "run_env[key]=$GITHUB_ACTION-$GITHUB_RUN_NUMBER-$GITHUB_RUN_ATTEMPT" \
-F "run_env[number]=$GITHUB_RUN_NUMBER" \
-F "run_env[branch]=$GITHUB_REF" \
-F "run_env[commit_sha]=$GITHUB_SHA" \
-F "run_env[url]=https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" \
https://analytics-api.buildkite.com/v1/uploads
3 changes: 3 additions & 0 deletions clients/typescript/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ build-dev: node_modules
tests:
pnpm run test

tests-CI:
pnpm run test-CI

style:
prettier --check --loglevel warn . && eslint src --quiet

Expand Down
44 changes: 43 additions & 1 deletion clients/typescript/ava.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
const [major, minor, _patch] = process.versions.node.split('.').map(Number)

// Developers can provide a `TEST_ONLY_DIALECT` value of `postgres`, `pglite`, or `sqlite`
// to run the unit tests only for that dialect.
// Developers can also provide a `DISABLE_DIALECT` value of `postgres`, `pglite`, or `sqlite`
// to disable the unit tests for that dialect but run all others.
const testOnlyDialect = process.env.TEST_ONLY_DIALECT
const disableDialect = process.env.DISABLE_DIALECT

if (testOnlyDialect && disableDialect) {
throw new Error(
'Cannot set both TEST_ONLY_DIALECT and DISABLE_DIALECT environment variables.'
)
}

let loaderArg
if (
major > 20 ||
Expand All @@ -11,9 +24,38 @@ if (
loaderArg = '--loader=tsx'
}

const files = ['test/**/*.test.ts', 'test/**/*.test.tsx']
const ignorePostgres = ['!test/**/postgres/**']
const ignorePglite = ['!test/**/pglite/**']
const ignoreSqlite = ['!test/**/sqlite/**']

switch (testOnlyDialect) {
case 'postgres':
files.push(...ignorePglite, ...ignoreSqlite)
break
case 'pglite':
files.push(...ignorePostgres, ...ignoreSqlite)
break
case 'sqlite':
files.push(...ignorePostgres, ...ignorePglite)
break
}

switch (disableDialect) {
case 'postgres':
files.push(...ignorePostgres)
break
case 'pglite':
files.push(...ignorePglite)
break
case 'sqlite':
files.push(...ignoreSqlite)
break
}

export default {
timeout: '10m',
files: ['test/**/*.test.ts', 'test/**/*.test.tsx'],
files,
extensions: {
ts: 'module',
tsx: 'module',
Expand Down
35 changes: 32 additions & 3 deletions clients/typescript/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@
"./op-sqlite": "./dist/drivers/op-sqlite/index.js",
"./generic": "./dist/drivers/generic/index.js",
"./node": "./dist/drivers/better-sqlite3/index.js",
"./node-postgres": "./dist/drivers/node-postgres/index.js",
"./pglite": "./dist/drivers/pglite/index.js",
"./react": "./dist/frameworks/react/index.js",
"./tauri-postgres": "./dist/drivers/tauri-postgres/index.js",
"./vuejs": "./dist/frameworks/vuejs/index.js",
"./wa-sqlite": "./dist/drivers/wa-sqlite/index.js",
"./tauri": "./dist/drivers/tauri-sqlite/index.js",
Expand Down Expand Up @@ -90,9 +93,18 @@
"node": [
"./dist/drivers/better-sqlite3/index.d.ts"
],
"node-postgres": [
"./dist/drivers/node-postgres/index.d.ts"
],
"pglite": [
"./dist/drivers/pglite/index.d.ts"
],
"react": [
"./dist/frameworks/react/index.d.ts"
],
"tauri-postgres": [
"./dist/drivers/tauri-postgres/index.d.ts"
],
"vuejs": [
"./dist/frameworks/vuejs/index.d.ts"
],
Expand Down Expand Up @@ -152,6 +164,7 @@
"build": "shx rm -rf dist && npm run build:copy-docker && concurrently \"tsup\" \"tsc -p tsconfig.build.json\" && node scripts/fix-imports.js",
"build:copy-docker": "shx mkdir -p ./dist/cli/docker-commands/docker && shx cp -r ./src/cli/docker-commands/docker ./dist/cli/docker-commands",
"test": "ava",
"test-CI": "DISABLE_DIALECT=postgres npm run test",
"generate-test-client": "npx tsx ./test/client/generateTestClient.ts",
"typecheck": "tsc -p tsconfig.json",
"posttest": "npm run typecheck",
Expand Down Expand Up @@ -202,6 +215,7 @@
"zod": "3.21.1"
},
"devDependencies": {
"@electric-sql/pglite": "^0.1.5",
"@electric-sql/prisma-generator": "workspace:*",
"@op-engineering/op-sqlite": ">= 2.0.16",
"@tauri-apps/plugin-sql": "2.0.0-alpha.5",
Expand All @@ -219,6 +233,7 @@
"@types/lodash.throttle": "^4.1.7",
"@types/lodash.uniqwith": "^4.5.9",
"@types/node": "^18.8.4",
"@types/pg": "^8.11.0",
"@types/prompts": "^2.4.9",
"@types/react": "^18.0.18",
"@types/tcp-port-used": "^1.0.2",
Expand All @@ -229,15 +244,17 @@
"@vue/test-utils": "^2.4.4",
"ava": "^4.3.1",
"concurrently": "^8.2.2",
"embedded-postgres": "16.1.1-beta.9",
"eslint": "^8.22.0",
"expo-sqlite": "^13.0.0",
"expo-sqlite": "^13.1.0",
"glob": "^10.3.10",
"global-jsdom": "24.0.0",
"husky": "^8.0.3",
"jsdom": "24.0.0",
"lint-staged": "^13.1.0",
"memorystorage": "^0.12.0",
"nodemon": "^3.0.2",
"pg": "^8.11.3",
"prettier": "2.8.2",
"react": "^18.2.0",
"react-dom": "^18.2.0",
Expand All @@ -255,9 +272,12 @@
},
"peerDependencies": {
"@capacitor-community/sqlite": ">= 5.6.2",
"@electric-sql/pglite": ">= 0.1.5",
"@op-engineering/op-sqlite": ">= 2.0.16",
"@tauri-apps/plugin-sql": "2.0.0-alpha.5",
"embedded-postgres": "16.1.1-beta.9",
"expo-sqlite": ">= 13.0.0",
"pg": "^8.11.3",
"prisma": "4.8.1",
"react": ">= 16.8.0",
"react-dom": ">= 16.8.0",
Expand All @@ -268,18 +288,27 @@
"zod": "3.21.1"
},
"peerDependenciesMeta": {
"@op-engineering/op-sqlite": {
"@capacitor-community/sqlite": {
"optional": true
},
"@capacitor-community/sqlite": {
"@electric-sql/pglite": {
"optional": true
},
"@op-engineering/op-sqlite": {
"optional": true
},
"@tauri-apps/plugin-sql": {
"optional": true
},
"embedded-postgres": {
"optional": true
},
"expo-sqlite": {
"optional": true
},
"pg": {
"optional": true
},
"prisma": {
"optional": true
},
Expand Down
Loading

0 comments on commit 450a65b

Please sign in to comment.