Implement aggregate functions for sparse vector. #476
Conversation
my-vegetable-has-exploded
force-pushed
the
sparse-agg
branch
from
1b265a6
to
421c71a
Compare
|
@usamoi PTAL |
|
Is this ready to merge? |
not yet. I am rewriting the state amd related function. |
|
@usamoi PTAL |
|
Ready to merge? |
|
ptal @usamoi |
|
Since it's hard to review, can @my-vegetable-has-exploded add more randomized tests and run multiple times locally to ensure the correctness? |
|
Both end2end and unit test |
Okay, I'll also look into how to make the code more readable. |
|
|
||
| # test avg(svector) get the same result as avg(vector) | ||
| query ? | ||
| SELECT avg(data) = avg(data::svector)::vector FROM test_vectors; |
There was a problem hiding this comment.
The elements from avg(data) is essentially within the range of [2,3], matching the expected value of the generated data ((100+1)/2*0.05) ~= 2.5.
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
my-vegetable-has-exploded
force-pushed
the
sparse-agg
branch
from
0fc1220
to
dde0d5d
Compare
Signed-off-by: my-vegetable-has-exploded <[email protected]>
my-vegetable-has-exploded
force-pushed
the
sparse-agg
branch
from
96c5e5a
to
723b475
Compare
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
my-vegetable-has-exploded
force-pushed
the
sparse-agg
branch
from
d21c223
to
6fd13de
Compare
| #[allow(unused_unsafe)] | ||
| unsafe { | ||
| pgrx::pg_sys::submodules::panic::pgrx_extern_c_guard(move || { | ||
| let mut agg_context: *mut ::pgrx::pg_sys::MemoryContextData = std::ptr::null_mut(); |
There was a problem hiding this comment.
pg will switch to a temporary memory context when call aggregate trans function.
https://github.com/postgres/postgres/blob/52b49b796cc7fd976f4da6aa49c9679ecdae8bd5/src/backend/executor/nodeAgg.c#L761-L801
doxygen
So we need to switch context like https://github.com/postgres/postgres/blob/7c655a04a2dc84b59ed6dce97bd38b79e734ecca/src/backend/utils/adt/numeric.c#L5635-L5648.
Currently pgrx's Aggregate trait can't pass a reference with lifetime. https://github.com/pgcentralfoundation/pgrx/blob/develop/pgrx/src/aggregate.rs. So I expand it. （ Of course, this piece of code is terrible.）
There was a problem hiding this comment.
There was a problem hiding this comment.
I feel not necessary for using Aggregate trait.
There was a problem hiding this comment.
I don't find a good way to swtich the context under pgrx currently.😭
There was a problem hiding this comment.
You could just write code in C way. pgrx is more a binding than a complete interface of PostgreSQL.
refs #475
Purpose of this pr is almost same with #463, but mainly for sparse vector.