postgrespro / pgsphere

PgSphere provides spherical data types, functions, operators, and indexing for PostgreSQL.
https://pgsphere.org
BSD 3-Clause "New" or "Revised" License
16 stars 14 forks source link

Mark pg_sphere functions as parallel safe? #7

Closed msdemlei closed 1 year ago

msdemlei commented 2 years ago

For all I can see everything we do ought to be parallel safe by postgres standards (i.e., we don't touch anything within the database at all).

Classic use cases (such as our healpix maps) profit greatly from parallel plans -- so, what about a round of declaring our functions PARALLEL SAFE?

The one problem I see at this point: How do we upgrade existing installations without going insane? Enumerating all the signatures in an endless ALTER FUNCTION sequence looks really unattractive to me...

obartunov commented 2 years ago

On Tue, May 17, 2022 at 1:38 PM msdemlei @.***> wrote:

For all I can see everything we do ought to be parallel safe by postgres standards (i.e., we don't touch anything within the database at all).

Classic use cases (such as our healpix maps) profit greatly from parallel plans -- so, what about a round of declaring our functions PARALLEL SAFE?

Do you have any benchmarks ?

The one problem I see at this point: How do we upgrade existing installations without going insane? Enumerating all the signatures in an endless ALTER FUNCTION sequence looks really unattractive to me...

Simple script with a bunch CREATE OR REPLACE

— Reply to this email directly, view it on GitHub https://github.com/postgrespro/pgsphere/issues/7, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQURYX6VHYA3SUTRQKD7ZDVKNZKJANCNFSM5WEORUYQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Postgres Professional: http://www.postgrespro.com The Russian Postgres Company

msdemlei commented 2 years ago

On Fri, May 27, 2022 at 04:02:10AM -0700, Oleg Bartunov wrote:

On Tue, May 17, 2022 at 1:38 PM msdemlei @.***> wrote:

Classic use cases (such as our healpix maps) profit greatly from parallel plans -- so, what about a round of declaring our functions PARALLEL SAFE?

Do you have any benchmarks ?

Nothing published for pg_sphere operations specifically, but anecdotically, what I wrote in https://blog.g-vo.org/parallel-queries.html applies; an all-sky healpix map of SDSS DR16 takes somewhat more than an hour on my server sequentially, and 15 minutes in the config mentioned in the blog post.

obartunov commented 2 years ago

On Mon, May 30, 2022 at 2:10 PM msdemlei @.***> wrote:

On Fri, May 27, 2022 at 04:02:10AM -0700, Oleg Bartunov wrote:

On Tue, May 17, 2022 at 1:38 PM msdemlei @.***> wrote:

Classic use cases (such as our healpix maps) profit greatly from parallel plans -- so, what about a round of declaring our functions PARALLEL SAFE?

Try this branch https://github.com/postgrespro/pgsphere/tree/parallel_safe

I was unable to test on my mac running Monterey 12.4 because of linking problem with libhealpix.

Do you have any benchmarks ?

Nothing published for pg_sphere operations specifically, but anecdotically, what I wrote in https://blog.g-vo.org/parallel-queries.html applies; an all-sky healpix map of SDSS DR16 takes somewhat more than an hour on my server sequentially, and 15 minutes in the config mentioned in the blog post.

— Reply to this email directly, view it on GitHub https://github.com/postgrespro/pgsphere/issues/7#issuecomment-1141023447, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQURYRXT6ZMB5XXB2KG24DVMSOZNANCNFSM5WEORUYQ . You are receiving this because you commented.Message ID: @.***>

-- Postgres Professional: http://www.postgrespro.com The Russian Postgres Company

msdemlei commented 2 years ago

On Mon, Jun 06, 2022 at 07:40:03AM -0700, Oleg Bartunov wrote:

Try this branch https://github.com/postgrespro/pgsphere/tree/parallel_safe

I was unable to test on my mac running Monterey 12.4 because of linking problem with libhealpix.

Looks good on a Debian stable, where it builds nicely, and the tests pass, too, except that init_test.out is slightly outdated:

-psql:pg_sphere.test.sql:9152: NOTICE: return type smoc is only a shell -psql:pg_sphere.test.sql:9158: NOTICE: argument type smoc is only a shell +psql:pg_sphere.test.sql:9154: NOTICE: return type smoc is only a shell +psql:pg_sphere.test.sql:9160: NOTICE: argument type smoc is only a shell

Technically, I wonder if the cleanup of the yacc-generated files shouldn't come in through a different branch/PR; the diff is rather large already with just the PARALLEL SAFE changes. But then if the individual commits survive the merge, it'll still be clear enough what happened the way things are.

Disclaimer: I've not tried to upgrade an existing database yet (will do once it's a PR). However, pg_sphere--1.2.0--1.2.1.sql.in runs nicely, so I'm confident it should work fine.

esabol commented 1 year ago

It's sad this has languished in a branch. Can we get some aspect of this submitted as a PR and merged?

vitcpp commented 1 year ago

@esabol We are planning to come to this task in the near future once we complete some other work. Now we have to do some work to be in sync with @msdemlei

esabol commented 1 year ago

This issue can be closed now, I think.