Re: [HACKERS] [PATCH] Call RelationDropStorage() for broader range of object drops.
To provide more context, in cstore_fdw creating the storage is easy, we only need to hook into CREATE FOREIGN TABLE using event triggers. Removing the storage is not that easy, for DROP FOREIGN TABLE we can use event triggers. But when we do DROP EXTENSION, the event triggers don't get fired (because they have already been dropped), so to handle DROP EXTENSION, we need to hook into the process utility hook. Now to implement this, (1) we get a list of all cstore tables (2) call postgres's utility hook, (3) if #2 succeeds clean-up all cstore table storage's. But when #3 happens the relation isn't there anymore, so we create a pseudo-relation [1] and call RelationDropStorage(). Implementing all of this seemed messy, so we thought maybe postgres could try to clean-up storage for every relation it assigns a relfilenode for. We are open to ideas. [1] https://github.com/citusdata/cstore_fdw/blob/store_data_in_internal_storage/cstore_fdw.c#L907
Re: [HACKERS] [PATCH] Call RelationDropStorage() for broader range of object drops.
On Wed, Sep 13, 2017 at 12:12 AM, Michael Paquier wrote: > > Foreign tables do not have physical storage assigned to by default. At > least heap_create() tells so, create_storage being set to false for a > foreign table. So there is nothing to clean up normally. Or is > cstore_fdw using directly heap_create with its own relfilenode set, > creating a physical storage? > cstore_fdw (in store_data_in_internal_storage branch) calls RelationCreateStorage() after CREATE FOREIGN TABLE completes [1]. Later it also creates the FSM fork and uses it for storing some metadata. [1] https://github.com/citusdata/cstore_fdw/blob/store_data_in_internal_storage/cstore_fdw.c#L237
[HACKERS] [PATCH] Call RelationDropStorage() for broader range of object drops.
Motivation for this patch is that some FDWs (notably, cstore_fdw) try
utilizing PostgreSQL internal storage. PostgreSQL assigns relfilenode's to
foreign tables, but doesn't clean up storage for foreign tables when
dropping tables. Therefore, in cstore_fdw we have to do some tricks to
handle dropping objects that lead to dropping of cstore table properly.
As far as I can see in the code, the requirement for
RelationDropStorage(rel) is having valid rel->rd_node.relNode, but it
doesn't actually require the storage files to actually exist. We don't emit
warning messages in mdunlinkfork() if the result of unlink() is ENOENT. So
we can RelationDropStorage() regardless of storage files existing or not,
given that the relation has valid relfilenode.
So I am suggesting to change the check at heap_drop_with_catalog() at
src/backend/catalog/heap.c:
-if (rel->rd_rel->relkind != RELKIND_VIEW &&
-rel->rd_rel->relkind != RELKIND_COMPOSITE_TYPE &&
-rel->rd_rel->relkind != RELKIND_FOREIGN_TABLE &&
-rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+if (OidIsValid(rel->rd_node.relNode))
{
RelationDropStorage(rel);
}
Any feedback on this?
Thanks,
Hadi
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 45ee9ac8b9..6ec2a98a99 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1828,10 +1828,7 @@ heap_drop_with_catalog(Oid relid)
/*
* Schedule unlinking of the relation's physical files at commit.
*/
- if (rel->rd_rel->relkind != RELKIND_VIEW &&
- rel->rd_rel->relkind != RELKIND_COMPOSITE_TYPE &&
- rel->rd_rel->relkind != RELKIND_FOREIGN_TABLE &&
- rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+ if (OidIsValid(rel->rd_node.relNode))
{
RelationDropStorage(rel);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] Make ExplainOpenGroup()/ExplainCloseGroup() available for extensions.
Hello Hackers,
The attached patch moves declarations of
ExplainOpenGroup()/ExplainCloseGroup() from explain.c to explain.h.
This can be useful for extensions that need explain groups in their
custom-scan explain output.
For example, Citus uses groups in its custom explain outputs [1]. But it
achieves it by having a copy of ExplainOpenGroup()/ExplainCloseGroup() in
its source code, which is not the best way.
Please review.
[1] https://github.com/citusdata/citus/blob/master/src/backend/
distributed/planner/multi_explain.c
Thanks,
Hadi
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 7648201218..46467e1045 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -124,10 +124,6 @@ static void ExplainCustomChildren(CustomScanState *css,
List *ancestors, ExplainState *es);
static void ExplainProperty(const char *qlabel, const char *value,
bool numeric, ExplainState *es);
-static void ExplainOpenGroup(const char *objtype, const char *labelname,
- bool labeled, ExplainState *es);
-static void ExplainCloseGroup(const char *objtype, const char *labelname,
- bool labeled, ExplainState *es);
static void ExplainDummyGroup(const char *objtype, const char *labelname,
ExplainState *es);
static void ExplainXMLTag(const char *tagname, int flags, ExplainState *es);
@@ -3213,7 +3209,7 @@ ExplainPropertyBool(const char *qlabel, bool value, ExplainState *es)
* If labeled is true, the group members will be labeled properties,
* while if it's false, they'll be unlabeled objects.
*/
-static void
+void
ExplainOpenGroup(const char *objtype, const char *labelname,
bool labeled, ExplainState *es)
{
@@ -3276,7 +3272,7 @@ ExplainOpenGroup(const char *objtype, const char *labelname,
* Close a group of related objects.
* Parameters must match the corresponding ExplainOpenGroup call.
*/
-static void
+void
ExplainCloseGroup(const char *objtype, const char *labelname,
bool labeled, ExplainState *es)
{
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 78822b766a..543b2bb0c6 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -101,4 +101,9 @@ extern void ExplainPropertyFloat(const char *qlabel, double value, int ndigits,
extern void ExplainPropertyBool(const char *qlabel, bool value,
ExplainState *es);
+extern void ExplainOpenGroup(const char *objtype, const char *labelname,
+ bool labeled, ExplainState *es);
+extern void ExplainCloseGroup(const char *objtype, const char *labelname,
+ bool labeled, ExplainState *es);
+
#endif /* EXPLAIN_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Make ExplainBeginGroup()/ExplainEndGroup() public.
Title should have been "Make ExplainOpenGroup()/ExplainCloseGroup() public.". Sorry for the misspell.
[HACKERS] [PATCH] Make ExplainBeginGroup()/ExplainEndGroup() public.
Hello,
The attached patch moves declarations of
ExplainBeginGroup()/ExplainEndGroup() from explain.c to explain.h.
This can be useful for extensions that need explain groups in their
custom-scan explain output.
For example, Citus uses groups in its custom explain outputs [1]. But it
achieves it by having a copy of
ExplainBeginGroup()/ExplainEndGroup() in its source code, which is not the
best way.
Please review.
[1]
https://github.com/citusdata/citus/blob/master/src/backend/distributed/planner/multi_explain.c
Thanks,
Hadi
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 7648201218..46467e1045 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -124,10 +124,6 @@ static void ExplainCustomChildren(CustomScanState *css,
List *ancestors, ExplainState *es);
static void ExplainProperty(const char *qlabel, const char *value,
bool numeric, ExplainState *es);
-static void ExplainOpenGroup(const char *objtype, const char *labelname,
- bool labeled, ExplainState *es);
-static void ExplainCloseGroup(const char *objtype, const char *labelname,
- bool labeled, ExplainState *es);
static void ExplainDummyGroup(const char *objtype, const char *labelname,
ExplainState *es);
static void ExplainXMLTag(const char *tagname, int flags, ExplainState *es);
@@ -3213,7 +3209,7 @@ ExplainPropertyBool(const char *qlabel, bool value, ExplainState *es)
* If labeled is true, the group members will be labeled properties,
* while if it's false, they'll be unlabeled objects.
*/
-static void
+void
ExplainOpenGroup(const char *objtype, const char *labelname,
bool labeled, ExplainState *es)
{
@@ -3276,7 +3272,7 @@ ExplainOpenGroup(const char *objtype, const char *labelname,
* Close a group of related objects.
* Parameters must match the corresponding ExplainOpenGroup call.
*/
-static void
+void
ExplainCloseGroup(const char *objtype, const char *labelname,
bool labeled, ExplainState *es)
{
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 78822b766a..543b2bb0c6 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -101,4 +101,9 @@ extern void ExplainPropertyFloat(const char *qlabel, double value, int ndigits,
extern void ExplainPropertyBool(const char *qlabel, bool value,
ExplainState *es);
+extern void ExplainOpenGroup(const char *objtype, const char *labelname,
+ bool labeled, ExplainState *es);
+extern void ExplainCloseGroup(const char *objtype, const char *labelname,
+ bool labeled, ExplainState *es);
+
#endif /* EXPLAIN_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Faster array_length()
Thanks for looking into this. > With that in mind, I was surprised that your test case showed any > improvement at all --- it looks like the arrays aren't getting compressed > for some reason. > > You are right, it seems that they were not getting compressed, probably because the arrays were "seq 1" which seems to not get compressed by pglz. When I changed the test data to an array containing 1 ones, there were no speed improvement anymore. I'll look into how to improve the compressed case and other issues you raised. Thanks, -- Hadi
[HACKERS] Faster array_length()
Hello,
The attached patch improves the performance of array_length() by detoasting
only the overhead part of the datum.
Here is a test case:
postgres=# create table array_length_test as select array_agg(a) a from
generate_series(1, 1) a, generate_series(1, 1) b group by b;
Without the patch:
postgres=# select sum(array_length(a, 1)) from array_length_test;
sum
---
1
(1 row)
Time: 199.002 ms
With the patch:
postgres=# select sum(array_length(a, 1)) from array_length_test;
sum
---
1
(1 row)
Time: 34.599 ms
The motivation for patch is that some of our customers use arrays to store
a sequence of tens of thousands of events in each row. They often need to
get the last 10 event for each row, for which we do A[array_length(A, 1) -
9: 100] (assuming 1M is an upper-bound. we could use array_length()
instead of this constant too, but that is unnecessary if we know the
upper-bound and only slows down the query). Without this optimization,
array gets detoasted twice. With this patch, array_length() becomes much
faster, and the whole query saves few seconds.
Of course this technique is applicable to some other functions too, but
they have never become a bottleneck for me, so I decided to keep the
changes only to this function.
Another alternative I could think of was introducing python style slicing,
in which negative indexes start from end of array, so -10 means 10th
element from end. I thought this would be a bigger change and is probably
unnecessary, so I decided to improve array_length() instead.
Feedback is welcome.
Thanks,
-- Hadi
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index 91df184..5e1d9c2 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -1719,11 +1719,15 @@ array_upper(PG_FUNCTION_ARGS)
Datum
array_length(PG_FUNCTION_ARGS)
{
- ArrayType *v = PG_GETARG_ARRAYTYPE_P(0);
+ Datum arrdatum = PG_GETARG_DATUM(0);
int reqdim = PG_GETARG_INT32(1);
+ ArrayType *v;
int *dimv;
int result;
+ v = (ArrayType *) PG_DETOAST_DATUM_SLICE(arrdatum, 0,
+ ARR_OVERHEAD_NONULLS(MAXDIM));
+
/* Sanity check: does it look like an array at all? */
if (ARR_NDIM(v) <= 0 || ARR_NDIM(v) > MAXDIM)
PG_RETURN_NULL();
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for Merge Join for Non '=' Operators
Hello Dilip, Query: select count(*) from t1,t2 where t1.b 12000; > > Test Result: > Nest Loop Join with Index Scan : 1653.506 ms > Sort Merge Join for (seq scan) : 610.257ms > > This looks like a great improvement. Repeating Nicolas's question, do you have a real-world example of such joins? In my experience, I see more queries like "self-join table A and table B where A.time BETWEEN B.time - '1 week' and B.time", similar to what Nicolas and Tom mentioned. As an example, "count users who placed an order in the week following their registration". Can you send a patch so we can also try it? Thanks, -- Hadi
Re: [HACKERS] A question about code in DefineRelation()
> > On second thought I noticed that that makes CREATE FOREIGN TABLE include > an OID column in newly-created foreign tables wrongly, when the > default_with_oids parameter is set to on. Please find attached a patch. > > The fix makes sense to me, since in ALTER TABLE SET WITH OIDS we check that the relation is a table and not a foreign table: 3160 case AT_AddOids: /* SET WITH OIDS */ 3161 ATSimplePermissions(rel, ATT_TABLE); So, I think we should be consistent between DefineRelation() and alter table. -- Hadi.
Re: [HACKERS] PostgreSQL Columnar Store for Analytic Workloads
Hi Stefan, On Tue, Apr 8, 2014 at 9:28 AM, Stefan Keller wrote: > Hi Hadi > > Do you think that cstore_fd*w* is also welll suited for storing and > retrieving linked data (RDF)? > I am not very familiar with RDF. Note that cstore_fdw doesn't change the query language of PostgreSQL, so if your queries are expressible in SQL, they can be answered using cstore_fdw too. If your data is huge and doesn't fit in memory, then using cstore_fdw can be beneficial for you. Can you give some more information about your use case? For example, what are some of your queries? do you have sample data? how much memory do you have? how large is the data? -- Hadi
[HACKERS] PostgreSQL Columnar Store for Analytic Workloads
Dear Hackers, We at Citus Data have been developing a columnar store extension for PostgreSQL. Today we are excited to open source it under the Apache v2.0 license. This columnar store extension uses the Optimized Row Columnar (ORC) format for its data layout, which improves upon the RCFile format developed at Facebook, and brings the following benefits: * Compression: Reduces in-memory and on-disk data size by 2-4x. Can be extended to support different codecs. We used the functions in pg_lzcompress.h for compression and decompression. * Column projections: Only reads column data relevant to the query. Improves performance for I/O bound queries. * Skip indexes: Stores min/max statistics for row groups, and uses them to skip over unrelated rows. We used the PostgreSQL FDW APIs to make this work. The extension doesn't implement the writable FDW API, but it uses the process utility hook to enable COPY command for the columnar tables. This extension uses PostgreSQL's internal data type representation to store data in the table, so this columnar store should support all data types that PostgreSQL supports. We tried the extension on TPC-H benchmark with 4GB scale factor on a m1.xlarge Amazon EC2 instance, and the query performance improved by 2x-3x compared to regular PostgreSQL table. Note that we flushed the page cache before each test to see the impact on disk I/O. When data is cached in memory, the performance of cstore_fdw tables were close to the performance of regular PostgreSQL tables. For more information, please visit: * our blog post: http://citusdata.com/blog/76-postgresql-columnar-store-for-analytics * our github page: https://github.com/citusdata/cstore_fdw Feedback from you is really appreciated. Thanks, -- Hadi
[HACKERS] PostgreSQL Columnar Store for Analytic Workloads
Dear Hackers, We at Citus Data have been developing a columnar store extension for PostgreSQL. Today we are excited to open source it under the Apache v2.0 license. This columnar store extension uses the Optimized Row Columnar (ORC) format for its data layout, which improves upon the RCFile format developed at Facebook, and brings the following benefits: * Compression: Reduces in-memory and on-disk data size by 2-4x. Can be extended to support different codecs. We used the functions in pg_lzcompress.h for compression and decompression. * Column projections: Only reads column data relevant to the query. Improves performance for I/O bound queries. * Skip indexes: Stores min/max statistics for row groups, and uses them to skip over unrelated rows. We used the PostgreSQL FDW APIs to make this work. The extension doesn't implement the writable FDW API, but it uses the process utility hook to enable COPY command for the columnar tables. This extension uses PostgreSQL's internal data type representation to store data in the table, so this columnar store should support all data types that PostgreSQL supports. We tried the extension on TPC-H benchmark with 4GB scale factor on a m1.xlarge Amazon EC2 instance, and the query performance improved by 2x-3x compared to regular PostgreSQL table. Note that we flushed the page cache before each test to see the impact on disk I/O. When data is cached in memory, the performance of cstore_fdw tables were close to the performance of regular PostgreSQL tables. For more information, please visit: * our blog post: http://citusdata.com/blog/76-postgresql-columnar-store-for-analytics * our github page: https://github.com/citusdata/cstore_fdw Feedback from you is really appreciated. Thanks, -- Hadi
[HACKERS] pglz_decompress()
Hello, The comments in pg_lzcompress.c say that: * If VARSIZE(x) == rawsize + sizeof(PGLZ_Header), then the data * is stored uncompressed as plain bytes. Thus, the decompressor * simply copies rawsize bytes from the location after the * header to the destination. But pg_lzdecompress doesn't seem to check explicitly for the VARSIZE(x) == rawsize + sizeof(PGLZ_Header) condition. Is it caller's responsibility to check for this condition before calling pg_lzdecompress(), or does it happen somehow in pg_lzdecompress()? Thanks, -- Hadi
[HACKERS] Cost estimation in foreign data wrappers
Hello, There is a callback function in fdw's which should also set estimates for startup and total costs for each path. Assume a fdw adds only one path (e.g. in file_fdw). I am trying to understand what can go wrong if we do a bad job in estimating these costs. Since we have only one scan path here, it doesn't make a difference in choosing the best scan path. By looking at the code and doing some experiments, I think this can be significant in (1) underestimating a nested loop's cost, (2) not materializing inner table in nested loop. * Are there any other cases that this can be significant? * Assume we are not sure about the exact cost, but we know that it is in [lower_bound, upper_bound] range, where upper_bound can be 10x lower_bound Then, what value is better to choose? lower bound? upper bound? or average? Thanks, -- Hadi
Re: [HACKERS] Improving avg performance for numeric
Hello, > int, float, double 26829 ms (26675 ms) -- 0.5% slower .. statistic error .. > cleaner code > numeric sum 6490 ms (7224 ms) -- 10% faster > numeric avg 6487 ms (12023 ms) -- 46% faster I also got very similar results. On the other hand, initially I was receiving sigsegv's whenever I wanted to try to run an aggregate function. gdb was telling that this was happening somewhere in nodeAgg.c at ExecInitAgg. While trying to find the reason, I had to reboot my computer at some point, after the reboot the sigsegv's went away. I want to look into this and find the reason, I think I have missed something here. Any thoughts about why this would happen? --Hadi -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improving avg performance for numeric
Hi Tom, Tom Lane wrote: > After thinking about that for awhile: if we pursue this type of > optimization, what would probably be appropriate is to add an aggregate > property (stored in pg_aggregate) that allows direct specification of > the size that the planner should assume for the aggregate's transition > value. We were getting away with a hardwired assumption of 8K for > "internal" because the existing aggregates that used that transtype all > had similar properties, but it was always really a band-aid not a proper > solution. A per-aggregate override could be useful in other cases too. Cool. I created a patch which adds an aggregate property to pg_aggregate, so the transition space is can be overridden. This patch doesn't contain the numeric optimizations. It uses "0" (meaning not-set) for all existing aggregates. I manual-tested it a bit, by changing this value for aggregates and observing the changes in plan. I also updated some docs and pg_dump. Does this look like something along the lines of what you meant? Thanks, -- Hadi aggregate-transspace.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improving avg performance for numeric
I am not sure how this works, but I also changed numeric sum(), and the views in question had a numeric sum() column. Can that have any impact? I am going to dig deeper to see why this happens. On Tue, Mar 19, 2013 at 6:25 PM, Tom Lane wrote: > Kevin Grittner writes: > > Hadi Moshayedi wrote: > >> I also noticed that this patch makes matview test fail. It seems > >> that it just changes the ordering of rows for queries like > >> "SELECT * FROM tv;". Does this seem like a bug in my patch, or > >> should we add "ORDER BY" clauses to this test to make it more > >> deterministic? > > > I added some ORDER BY clauses. That is probably a good thing > > anyway for purposes of code coverage. Does that fix it for you? > > Uh, what? Fooling around with the implementation of avg() should surely > not change any planning decisions. > > regards, tom lane >
Re: [HACKERS] Improving avg performance for numeric
Hello, I updated the patch by taking ideas from your patch, and unifying the transition struct and update function for different aggregates. The speed of avg improved even more. It now has 60% better performance than the current committed version. This patch optimizes numeric/int8 sum, avg, stddev_pop, stddev_samp, var_pop, var_samp. I also noticed that this patch makes matview test fail. It seems that it just changes the ordering of rows for queries like "SELECT * FROM tv;". Does this seem like a bug in my patch, or should we add "ORDER BY" clauses to this test to make it more deterministic? I also agree with you that adding sum functions to use preallocated buffers will make even more optimization. I'll try to see if I can find a simple way to do this. Thanks, -- Hadi On Mon, Mar 18, 2013 at 5:25 PM, Pavel Stehule wrote: > Hello > > I played with sum(numeric) optimization > > Now it is based on generic numeric_add function - this code is > relative old - now we can design aggregates with internal transition > buffers, and probably we can do this work more effective. > > I just removed useles palloc/free operations and I got a 30% better > performance! My patch is ugly - because I used a generic add_var > function. Because Sum, Avg and almost all aggregates functions is > limited by speed of sum calculation I thing so we need a new numeric > routines optimized for calculation "sum", that use a only preallocated > buffers. A speed of numeric is more important now, because there are > more and more warehouses, where CPU is botleneck. > > Regards > > Pavel > > > 2013/3/18 Hadi Moshayedi : > > Hi Pavel, > > > > Thanks a lot for your feedback. > > > > I'll work more on this patch this week, and will send a more complete > patch > > later this week. > > > > I'll also try to see how much is the speed up of this method for other > > types. > > > > Thanks, > > -- Hadi > > > > > > On Mon, Mar 18, 2013 at 10:36 AM, Pavel Stehule > > > wrote: > >> > >> 2013/3/16 Hadi Moshayedi : > >> > Revisiting: > >> > http://www.postgresql.org/message-id/45661be7.4050...@paradise.net.nz > >> > > >> > I think the reasons which the numeric average was slow were: > >> > (1) Using Numeric for count, which is slower than int8 to increment, > >> > (2) Constructing/deconstructing arrays at each transition step. > >> > > >> > This is also discussed at: > >> > > >> > > http://www.citusdata.com/blog/53-postgres-performance-to-avg-or-to-sum-divided-by-count > >> > > >> > So, I think we can improve the speed of numeric average by keeping the > >> > transition state as an struct in the aggregate context, and just > passing > >> > the > >> > pointer to that struct from/to the aggregate transition function. > >> > > >> > The attached patch uses this method. > >> > > >> > I tested it using the data generated using: > >> > CREATE TABLE avg_test AS SELECT (random() * 999)::decimal(5,2) as d > FROM > >> > generate_series(1, 1000) s; > >> > > >> > After applying this patch, run time of "SELECT avg(d) FROM avg_test;" > >> > improves from 10.701 seconds to 5.204 seconds, which seems to be a > huge > >> > improvement. > >> > > >> > I think we may also be able to use a similar method to improve > >> > performance > >> > of some other numeric aggregates (like stddev). But I want to know > your > >> > feedback first. > >> > > >> > Is this worth working on? > >> > >> I checked this patch and it has a interesting speedup - and a price of > >> this methoud should not be limited to numeric type only > >> > >> Pavel > >> > >> > > >> > Thanks, > >> > -- Hadi > >> > > >> > > >> > > >> > -- > >> > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > >> > To make changes to your subscription: > >> > http://www.postgresql.org/mailpref/pgsql-hackers > >> > > > > > > numeric-optimize-v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improving avg performance for numeric
Hi Pavel, Thanks a lot for your feedback. I'll work more on this patch this week, and will send a more complete patch later this week. I'll also try to see how much is the speed up of this method for other types. Thanks, -- Hadi On Mon, Mar 18, 2013 at 10:36 AM, Pavel Stehule wrote: > 2013/3/16 Hadi Moshayedi : > > Revisiting: > > http://www.postgresql.org/message-id/45661be7.4050...@paradise.net.nz > > > > I think the reasons which the numeric average was slow were: > > (1) Using Numeric for count, which is slower than int8 to increment, > > (2) Constructing/deconstructing arrays at each transition step. > > > > This is also discussed at: > > > http://www.citusdata.com/blog/53-postgres-performance-to-avg-or-to-sum-divided-by-count > > > > So, I think we can improve the speed of numeric average by keeping the > > transition state as an struct in the aggregate context, and just passing > the > > pointer to that struct from/to the aggregate transition function. > > > > The attached patch uses this method. > > > > I tested it using the data generated using: > > CREATE TABLE avg_test AS SELECT (random() * 999)::decimal(5,2) as d FROM > > generate_series(1, 1000) s; > > > > After applying this patch, run time of "SELECT avg(d) FROM avg_test;" > > improves from 10.701 seconds to 5.204 seconds, which seems to be a huge > > improvement. > > > > I think we may also be able to use a similar method to improve > performance > > of some other numeric aggregates (like stddev). But I want to know your > > feedback first. > > > > Is this worth working on? > > I checked this patch and it has a interesting speedup - and a price of > this methoud should not be limited to numeric type only > > Pavel > > > > > Thanks, > > -- Hadi > > > > > > > > -- > > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > > To make changes to your subscription: > > http://www.postgresql.org/mailpref/pgsql-hackers > > >
[HACKERS] Improving avg performance for numeric
Revisiting: http://www.postgresql.org/message-id/45661be7.4050...@paradise.net.nz I think the reasons which the numeric average was slow were: (1) Using Numeric for count, which is slower than int8 to increment, (2) Constructing/deconstructing arrays at each transition step. This is also discussed at: http://www.citusdata.com/blog/53-postgres-performance-to-avg-or-to-sum-divided-by-count So, I think we can improve the speed of numeric average by keeping the transition state as an struct in the aggregate context, and just passing the pointer to that struct from/to the aggregate transition function. The attached patch uses this method. I tested it using the data generated using: CREATE TABLE avg_test AS SELECT (random() * 999)::decimal(5,2) as d FROM generate_series(1, 1000) s; After applying this patch, run time of "SELECT avg(d) FROM avg_test;" improves from 10.701 seconds to 5.204 seconds, which seems to be a huge improvement. I think we may also be able to use a similar method to improve performance of some other numeric aggregates (like stddev). But I want to know your feedback first. Is this worth working on? Thanks, -- Hadi numeric-avg-optimize.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
