Use Insert unnest to improve performance - #435
Conversation
| let rec coerceSchema = schema => | ||
| switch schema->S.classify { | ||
| | BigInt => BigInt.schema->S.toUnknown | ||
| | Option(child) | ||
| | Null(child) => | ||
| S.null(child->coerceSchema)->S.toUnknown | ||
| | Array(child) => { | ||
| hasArrayField := true | ||
| S.array(child->coerceSchema)->S.toUnknown | ||
| } | ||
| | JSON(_) => { | ||
| hasArrayField := true | ||
| schema | ||
| } | ||
| | Bool => | ||
| // Workaround for https://github.com/porsager/postgres/issues/471 | ||
| S.union([ | ||
| S.literal("t")->S.to(_ => true), | ||
| S.literal("f")->S.to(_ => false), | ||
| ])->S.toUnknown | ||
| | _ => schema | ||
| } |
There was a problem hiding this comment.
Not sure of the compiler optimisations but this fn assignment should probably be hoisted to a higher scope than inside the iteration
| S.array(child->coerceSchema)->S.toUnknown | ||
| } | ||
| | JSON(_) => { | ||
| hasArrayField := true |
There was a problem hiding this comment.
Why does json mean array? Or is it just that they are treated the same?
There was a problem hiding this comment.
Because a JSON value might be an array, in this case it'll be flattened inside of the unnest function.
|
|
||
| Async.it("All type entity without array types for unnest case", async () => { |
There was a problem hiding this comment.
It would be nice to see a test showing the sql query that gets built on a given entity
There was a problem hiding this comment.
It shouldn't be difficult. I need to split the code a little bit.
| switch field { | ||
| | Field(f) => | ||
| switch f.fieldType { | ||
| | Custom(fieldType) => `${(Text :> string)}[]::${(fieldType :> string)}` |
There was a problem hiding this comment.
I don't really understand what's happening here with Custom type any chance you can add a note?
There was a problem hiding this comment.
We cast a text type to enum type, since postgres can't do it automatically.
| // Workaround for https://github.com/porsager/postgres/issues/471 | ||
| S.union([ | ||
| S.literal("t")->S.to(_ => true), | ||
| S.literal("f")->S.to(_ => false), | ||
| ])->S.toUnknown |
There was a problem hiding this comment.
Am I understanding correctly that the whole purpose of this function is to transform booleans and flag in the case an array/json type exists? But still to keep the same underlying schema otherwise?
There was a problem hiding this comment.
Yes, besides the part of array/json. array/json has nothing to do with it.
- We always convert bool values, since before it was done by
sqlcall - Array/json check needed to decide whether we want to use
insert unnestor fallback to the old query.
| ~getKey=({chainId, eventId}) => { | ||
| chainId, | ||
| eventId, | ||
| eventId: eventId->BigInt.toString, |
There was a problem hiding this comment.
Why did this type change?
There was a problem hiding this comment.
So the schema field matches the table field.
| # We are using insert unnest for the case | ||
| # So test it as well | ||
| type EntityWithAllNonArrayTypes { |
There was a problem hiding this comment.
We should test one that also uses an enum and another entity
JonoPrest
left a comment
JonoPrest
left a comment
There was a problem hiding this comment.
Hey Dmitry, I don't want to block the release but I think the main points for me are to see the custom types in the test and it would be very helpful to see the different branches of the sql query represented in a test so it's easy to understand.
If you need to fast track this I will approve so you can merge when you feel it's ready 👍🏼
a54ab37 to
99a862d
Compare
2 participants
Final results: 30% DB Write decrease. For the case with raw_events it's only 2%.
If we are optimistic, on my machine 30% of DB Write is 3% of the total time.
Also,
unnestdoesn't work with array fields, so I had to fallback to the old approach if that's the case.