search

lanterndata / lantern

PostgreSQL vector database extension for building AI applications
https://lantern.dev
GNU Affero General Public License v3.0
790 stars 56 forks source link

Fix wrong memory context for allocation in op_rewrite.c #231

Closed var77 closed 11 months ago

var77 commented 11 months ago

The nodes were being allocated in MessageContext which doesn't live long enough and in some cases it was causing segfault when postgres was trying to access the nodes. Per source-code the plan tree is being allocated in PortalContext, so we also should allocate the overwritten nodes in PortalContext

github-actions[bot] commented 11 months ago

Benchmarks

metric old new pct change
recall (after create) 0.740 0.740 -
recall (after insert) 0.756 0.756 -
select bulk tps 481.514 493.884 +2.57%
select bulk latency (ms) 15.964 15.513 -2.83%
select bulk latency (stddev ms) 3.298 2.926 -11.28%
create latency (ms) 1217.728 1191.480 -2.16%
insert bulk tps 11.006 11.393 +3.51%
insert bulk latency (ms) 90.850 87.766 -3.39%
insert bulk latency (stddev ms) 3.735 2.286 -38.80%
disk usage (bytes) 6348800.000 6348800.000 -
codecov[bot] commented 11 months ago

Codecov Report

Merging #231 (1407891) into main (b90926c) will not change coverage. The diff coverage is 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #231 +/- ## ======================================= Coverage 78.20% 78.20% ======================================= Files 23 23 Lines 1647 1647 Branches 405 405 ======================================= Hits 1288 1288 Misses 178 178 Partials 181 181 ``` | [Files](https://app.codecov.io/gh/lanterndata/lantern/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lanterndata) | Coverage Δ | | |---|---|---| | [src/hooks/op\_rewrite.c](https://app.codecov.io/gh/lanterndata/lantern/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lanterndata#diff-c3JjL2hvb2tzL29wX3Jld3JpdGUuYw==) | `83.94% <100.00%> (ø)` | |
Ngalstyan4 commented 11 months ago

We should check and see whether the same change must be done here as well or no.

In there we a new Function Expression node.

var77 commented 11 months ago

Sorry actually I made the change in this PR for Function Expression, the on for List Node is here