Implement aggregate functions for sparse vector. #476

my-vegetable-has-exploded · 2024-05-12T11:39:42Z

refs #475

Purpose of this pr is almost same with #463, but mainly for sparse vector.

src/sql/finalize.sql

VoVAllen · 2024-05-14T05:25:30Z

@usamoi PTAL

VoVAllen · 2024-05-15T19:15:59Z

Is this ready to merge?

my-vegetable-has-exploded · 2024-05-16T02:23:15Z

Is this ready to merge?

not yet. I am rewriting the state amd related function.

src/datatype/functions_svecf32.rs

VoVAllen · 2024-05-18T05:57:21Z

@usamoi PTAL

src/sql/finalize.sql

src/datatype/functions_svecf32.rs

VoVAllen · 2024-05-20T07:22:16Z

Ready to merge?

my-vegetable-has-exploded · 2024-05-20T07:39:44Z

ptal @usamoi

usamoi

It looks fine but too complex to review.

VoVAllen · 2024-05-21T22:39:51Z

Since it's hard to review, can @my-vegetable-has-exploded add more randomized tests and run multiple times locally to ensure the correctness?

VoVAllen · 2024-05-21T22:40:09Z

Both end2end and unit test

my-vegetable-has-exploded · 2024-05-22T01:36:11Z

Since it's hard to review, can @my-vegetable-has-exploded add more randomized tests and run multiple times locally to ensure the correctness?

Okay, I'll also look into how to make the code more readable.

my-vegetable-has-exploded · 2024-05-22T14:13:51Z

tests/sqllogictest/svector.slt

+
+# test avg(svector) get the same result as avg(vector)
+query ?
+SELECT avg(data) = avg(data::svector)::vector FROM test_vectors;


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]>

my-vegetable-has-exploded · 2024-05-27T15:15:08Z

src/datatype/aggregate_svecf32.rs

+    #[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();


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.）

pgcentralfoundation/pgrx#1719

I feel not necessary for using Aggregate trait.

I don't find a good way to swtich the context under pgrx currently.😭

You could just write code in C way. pgrx is more a binding than a complete interface of PostgreSQL.

You could just write code in C way. pgrx is more a binding than a complete interface of PostgreSQL.

I add a macro to wrap it currently.

Signed-off-by: my-vegetable-has-exploded <[email protected]>

src/datatype/aggregate_svecf32.rs

src/sql/finalize.sql

src/datatype/mod.rs

crates/base_macros/src/lib.rs

Signed-off-by: my-vegetable-has-exploded <[email protected]>

my-vegetable-has-exploded · 2024-06-04T06:32:23Z

ptal @usamoi

src/datatype/aggregate_vecf32.rs

src/datatype/aggregate_svecf32.rs

Signed-off-by: my-vegetable-has-exploded <[email protected]>

usamoi · 2024-06-04T07:55:34Z

src/datatype/aggregate_svecf32.rs

+                new_state.merge_in_place(SVecf32Borrowed::new(
+                    dims as u32,
+                    state.indexes(),
+                    state.values(),
+                ));


state may not be a SVecf32 because of zeros. merge_in_place should not accept SVecf32Borrowed.

I think zero will be filter in Line115.

So what filter_zero is used for?

Mainly for avg aggregate. Elements in state may be get zero after division by count.

usamoi · 2024-06-04T08:07:32Z

src/datatype/aggregate_svecf32.rs

+fn _vectors_svecf32_aggregate_sum_finalfunc(state: Internal) -> Option<SVecf32Output> {
+    match unsafe { state.get_mut::<SVecf32AggregateAvgSumStype>() } {
+        Some(state) => {
+            state.filter_zero();


So it can be safely removed.

It should be reasonable.

my-vegetable-has-exploded commented May 12, 2024

View reviewed changes

src/sql/finalize.sql Outdated Show resolved Hide resolved

my-vegetable-has-exploded commented May 12, 2024

View reviewed changes

src/sql/finalize.sql Outdated Show resolved Hide resolved

my-vegetable-has-exploded force-pushed the sparse-agg branch from 1b265a6 to 421c71a Compare May 12, 2024 11:52

VoVAllen requested a review from usamoi May 14, 2024 05:25

my-vegetable-has-exploded commented May 16, 2024

View reviewed changes

src/datatype/functions_svecf32.rs Outdated Show resolved Hide resolved

my-vegetable-has-exploded marked this pull request as draft May 17, 2024 07:46

my-vegetable-has-exploded marked this pull request as ready for review May 17, 2024 08:00

my-vegetable-has-exploded commented May 18, 2024

View reviewed changes

src/sql/finalize.sql Outdated Show resolved Hide resolved

usamoi reviewed May 19, 2024

View reviewed changes

usamoi previously approved these changes May 20, 2024

View reviewed changes

my-vegetable-has-exploded dismissed usamoi’s stale review via 0fc1220 May 22, 2024 14:08

my-vegetable-has-exploded commented May 22, 2024

View reviewed changes

my-vegetable-has-exploded added 9 commits May 23, 2024 11:37

Implement aggregate functions for sparse vector.

38f508f

Signed-off-by: my-vegetable-has-exploded <[email protected]>

fix NULL.

2a11b64

Signed-off-by: my-vegetable-has-exploded <[email protected]>

use iter for div.

f98eb9d

Signed-off-by: my-vegetable-has-exploded <[email protected]>

use null for initcond.

20af580

Signed-off-by: my-vegetable-has-exploded <[email protected]>

Implement sparse accumulate state.

1089e0d

Signed-off-by: my-vegetable-has-exploded <[email protected]>

add more tests.

c38ebdd

Signed-off-by: my-vegetable-has-exploded <[email protected]>

fix replace & clippy.

c6881cb

Signed-off-by: my-vegetable-has-exploded <[email protected]>

Simplify & fix zero panic.

337c486

Signed-off-by: my-vegetable-has-exploded <[email protected]>

add random test for aggregate.

396871e

Signed-off-by: my-vegetable-has-exploded <[email protected]>

my-vegetable-has-exploded added 2 commits May 24, 2024 07:14

fix: rename state.

4695e38

Signed-off-by: my-vegetable-has-exploded <[email protected]>

fix: rename state.

99b47d4

Signed-off-by: my-vegetable-has-exploded <[email protected]>

my-vegetable-has-exploded marked this pull request as draft May 25, 2024 14:00

my-vegetable-has-exploded added 2 commits May 27, 2024 15:01

rm toast & use internal as stype.

dffce45

Signed-off-by: my-vegetable-has-exploded <[email protected]>

use internal type & switch memory context.

6fd13de

Signed-off-by: my-vegetable-has-exploded <[email protected]>

my-vegetable-has-exploded force-pushed the sparse-agg branch from d21c223 to 6fd13de Compare May 27, 2024 15:03

my-vegetable-has-exploded commented May 27, 2024

View reviewed changes

fix capacity check.

999652d

Signed-off-by: my-vegetable-has-exploded <[email protected]>

my-vegetable-has-exploded mentioned this pull request May 28, 2024

feat: temporary support for sparse aggregate. #489

Merged

my-vegetable-has-exploded added 3 commits May 31, 2024 11:58

use macro to switch context.

40e7f61

Signed-off-by: my-vegetable-has-exploded <[email protected]>

Merge remote-tracking branch 'origin/main' into sparse-agg

2a9e087

fix merge.

53bae90

Signed-off-by: my-vegetable-has-exploded <[email protected]>

my-vegetable-has-exploded marked this pull request as ready for review May 31, 2024 12:30

my-vegetable-has-exploded commented Jun 1, 2024

View reviewed changes

src/datatype/aggregate_svecf32.rs Outdated Show resolved Hide resolved

usamoi reviewed Jun 1, 2024

View reviewed changes

src/sql/finalize.sql Outdated Show resolved Hide resolved

usamoi reviewed Jun 1, 2024

View reviewed changes

src/datatype/mod.rs Outdated Show resolved Hide resolved

usamoi reviewed Jun 1, 2024

View reviewed changes

crates/base_macros/src/lib.rs Outdated Show resolved Hide resolved

usamoi mentioned this pull request Jun 3, 2024

chore: update install script #492

Merged

my-vegetable-has-exploded added 3 commits June 3, 2024 12:45

Merge remote-tracking branch 'origin/main' into sparse-agg

68059ac

switch context in c way and rm option of internal.

6a22145

Signed-off-by: my-vegetable-has-exploded <[email protected]>

fix context.

f8bcbb1

Signed-off-by: my-vegetable-has-exploded <[email protected]>

usamoi reviewed Jun 4, 2024

View reviewed changes

rm useless codes.

c3c16cf

Signed-off-by: my-vegetable-has-exploded <[email protected]>

usamoi reviewed Jun 4, 2024

View reviewed changes

usamoi approved these changes Jun 4, 2024

View reviewed changes

usamoi added this pull request to the merge queue Jun 4, 2024

Merged via the queue into tensorchord:main with commit d7d8d0b Jun 4, 2024
12 checks passed

Implement aggregate functions for sparse vector. #476

Implement aggregate functions for sparse vector. #476

Uh oh!

Conversation

my-vegetable-has-exploded commented May 12, 2024

Uh oh!

Uh oh!

Uh oh!

VoVAllen commented May 14, 2024

Uh oh!

VoVAllen commented May 15, 2024

Uh oh!

my-vegetable-has-exploded commented May 16, 2024

Uh oh!

Uh oh!

VoVAllen commented May 18, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VoVAllen commented May 20, 2024

Uh oh!

my-vegetable-has-exploded commented May 20, 2024

Uh oh!

usamoi left a comment

Choose a reason for hiding this comment

Uh oh!

VoVAllen commented May 21, 2024

Uh oh!

VoVAllen commented May 21, 2024

Uh oh!

my-vegetable-has-exploded commented May 22, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

my-vegetable-has-exploded commented Jun 4, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

my-vegetable-has-exploded Jun 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

my-vegetable-has-exploded Jun 4, 2024 •

edited

Loading