-
Notifications
You must be signed in to change notification settings - Fork 96
feat(zero): Treat PG Arrays as JSON #4390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Branch | arv/pg-array-type |
Testbed | Linux |
Click to view all benchmark results
Benchmark | File Size | Benchmark Result kilobytes (KB) (Result Δ%) | Upper Boundary kilobytes (KB) (Limit %) |
---|---|---|---|
zero-package.tgz | 📈 view plot 🚷 view threshold | 1,166.26 KB(+0.03%)Baseline: 1,165.93 KB | 1,189.25 KB (98.07%) |
zero.js | 📈 view plot 🚷 view threshold | 193.82 KB(0.00%)Baseline: 193.82 KB | 197.70 KB (98.04%) |
zero.js.br | 📈 view plot 🚷 view threshold | 54.49 KB(+0.06%)Baseline: 54.45 KB | 55.54 KB (98.10%) |
|
Branch | arv/pg-array-type |
Testbed | Linux |
Click to view all benchmark results
Benchmark | Throughput | Benchmark Result operations / second (ops/s) (Result Δ%) | Lower Boundary operations / second (ops/s) (Limit %) |
---|---|---|---|
src/client/custom.bench.ts > big schema | 📈 view plot 🚷 view threshold | 361,973.61 ops/s(+26.21%)Baseline: 286,792.59 ops/s | 122,455.16 ops/s (33.83%) |
src/client/zero.bench.ts > basics > All 1000 rows x 10 columns (numbers) | 📈 view plot 🚷 view threshold | 1,576.00 ops/s(+19.00%)Baseline: 1,324.42 ops/s | 593.26 ops/s (37.64%) |
src/client/zero.bench.ts > pk compare > pk = N | 📈 view plot 🚷 view threshold | 30,640.00 ops/s(+19.99%)Baseline: 25,535.97 ops/s | 12,187.31 ops/s (39.78%) |
src/client/zero.bench.ts > with filter > Lower rows 500 x 10 columns (numbers) | 📈 view plot 🚷 view threshold | 2,536.48 ops/s(+21.58%)Baseline: 2,086.19 ops/s | 920.98 ops/s (36.31%) |
Do we want to store in SQLite as json to support future JSON operators? |
Filters in zero are often delegated up to db so any json operators we support we will want to ideally run against SQLite too. |
It doesn't matter. SQLite is dynamically typed and TEXT and JSON are the same. The only real difference is that ORMs might do something special for JSON columns. |
// in current and export it appropriately as the new version | ||
// in `mod.ts`. | ||
t(current, '2v7p7445ss8ce', '/changes/v0/stream'); | ||
t(current, 'hfkz08pmaavz', '/changes/v0/stream'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ColumnSpec got a new optional field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still reviewing but very cool. How far down the priority list are array operators?
.columns({ | ||
id: string(), | ||
name: string(), | ||
nameArray: json<string[]>(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is an array of json possible in PG? E.g., json<json[]>
. Granted it seems like an insane edge case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From zero's point of view it is just json. I haven't tested if that maps cleanly... Or at least I don't remember
const elemPgTypeClass = isArray(col.type) | ||
? isEnum(col.type) | ||
? PostgresTypeClass.Enum | ||
: PostgresTypeClass.Base |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Base is the super type of all type in PG or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not entirely correct. I should add a comment... there are more types and this fells a bit unclean
// If the column is an array, this will be the type of the | ||
// elements in the array. If the column is not an array, | ||
// this will be null. | ||
elemPgTypeClass: pgTypeClassSchema.nullable().optional(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and we've punted on supporting arrays of arrays, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pg does not support array of array. it supports multi dimensional arrays which is slightly different. Those are strange in pg to say the least
type NameMapper, | ||
} from '../../../zero-schema/src/name-mapper.ts'; | ||
import type {Writable} from '../../../shared/src/writable.ts'; | ||
import type {TableSchema} from '../../../zero-schema/src/table-schema.ts'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does your prettier plugin auto-sort imports?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I'm not the only one who has organize imports on save...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not prettier... It is vscode
Support for Postgres array types
This PR introduces support for Postgres array types in our schema system. Here's what's included:
Key changes
|TEXT_ARRAY
attribute, using the same pattern as our existing attributes for enums and non-null typesImplementation details
ServerColumnSchema
to include anisArray
flag alongside the existingisEnum
flagdataTypeToZqlValueType
to handle array types properlyisEnum()
andisArray()
for type checkingChallenges addressed
The main complexity was properly handling enum arrays, which required detecting:
This required querying the Postgres type system more deeply to get information about element types.
Testing
elemPgTypeClass: null
field