versus this:
> a = [{}, [], null, true, 1, 'SomeOtherString']
[ {}, [], null, true, 1, 'SomeOtherString' ]
> a.sort().reverse()
[ true, null, {}, 'SomeOtherString', 1, [] ]
and this:
> a = [{}, [], null, true, 1, '2']
[ {}, [], null, true, 1, '2' ]
> a.sort().reverse()
[ true, null, {}, '2', 1, [] ]
So we can't replicate javascript sort order without emulating those.
Regards,
Ali Akbar
y client can exploit some segfault bug like the one in perl Dmitry
mentoined upthread, and the postgresql service is down.
Note: killing the server process with pg_terminate_backend isn't causing
this behavior to happen. The client reconnects normally, and the service is
still running.
Regards,
Ali Akbar
2015-01-20 18:17 GMT+07:00 Ali Akbar :
> 2015-01-20 14:22 GMT+07:00 Jeff Davis :
>
>> The current patch, which I am evaluating for commit, does away with
>> per-group memory contexts (it uses a common context for all groups), and
>> reduces the initial array allocation fro
eant to be freed by destroying
+ * the parent context.
+ *
* astate is working state (must not be NULL)
* rcontext is where to construct result
*/
Regards,
--
Ali Akbar
2015-01-18 10:44 GMT+07:00 Peter Eisentraut :
> On 1/7/15 8:51 PM, Ali Akbar wrote:
> > So now +1 for back-patching this.
>
> Done. Was a bit of work to get it into all the old versions.
>
Wow. Thanks.
Btw, for bug-fix patches like this, should the patch creator (me) also
keep references to such values in
> > your user_fctx, you must either copy them into the
> > multi_call_memory_ctx after detoasting, or ensure
> > that you detoast the values only in that context.
> >
> >
>
> I'm OK with this.
at's negligible.
> And everyone who calls initArrayResultArr() will get the error handling
> for free.
>
> Patch v7 attached, implementing those two changes, i.e.
>
> * makeMdArrayResult(..., astate->private_cxt)
> * move error handling into initArrayResultArr()
> * remove element_type from initArrayResultArr() signature
>
Reviewing the v7 patch:
- applies cleanly to current master. patch format, whitespace, etc is good
- make check runs without error
- performance & memory usage still consistent
If you think we don't have to add the comment (see above), i'll mark this
as ready for committer
Regards,
--
Ali Akbar
In the CF, the status becomes "Needs Review". Let's continue our discussion
of makeArrayResult* behavior if subcontext=false and release=true (more
below):
2014-12-22 8:08 GMT+07:00 Ali Akbar :
>
> With this API, i think we should make it clear if we call initArrayResult
&g
space correctly, so if '1'
becomes 'http://test.com/a' >1', there should be no
issue in those applications.
So now +1 for back-patching this.
Regards,
--
Ali Akbar
atch from Tomas only change initArrayResult* functions. initArrayResult
is new API in 9.5 (commit bac2739), with old API still works as-is.
--
Ali Akbar
The API user must choose between
release=true, wasting cycles but preventing memory leak, or managing memory
from the parent memory context.
In one possible use case, for efficiency maybe the caller will create a
special parent memory context for all array accumulation. Then uses
makeArrayResult* with release=false, and in the end releasing the parent
memory context once for all.
Regards,
--
Ali Akbar
2014-12-16 11:01 GMT+07:00 Ali Akbar :
>
>
>
> 2014-12-16 10:47 GMT+07:00 Ali Akbar :
>>
>>
>> 2014-12-16 6:27 GMT+07:00 Tomas Vondra :
>> Just fast-viewing the patch.
>>
>> The patch is not implementing the checking for not creating new con
2014-12-16 10:47 GMT+07:00 Ali Akbar :
>
>
> 2014-12-16 6:27 GMT+07:00 Tomas Vondra :
>>
>> On 15.12.2014 22:35, Jeff Janes wrote:
>> > On Sat, Nov 29, 2014 at 8:57 AM, Tomas Vondra > > <mailto:t...@fuzzy.cz>> wrote:
>> >
>> > H
;:
> > plperl.c:1196: error: too few arguments to function 'accumArrayResult'
> > plperl.c: In function 'plperl_array_to_datum':
> > plperl.c:1223: error: too few arguments to function 'initArrayResult'
> >
> > Cheers,
>
> Thanks, attached is a version that fixes this.
>
Just fast-viewing the patch.
The patch is not implementing the checking for not creating new context in
initArrayResultArr. I think we should implement it also there for
consistency (and preventing future problems).
Regards,
--
Ali Akbar
ive until the SRF is finished running. In most cases, this means
> that you should switch into multi_call_memory_ctx while doing the
> first-call setup.
>
The important part "However, if you want to allocate any data structures to
live across calls, you need to put them somewhere else." is buried in the
docs.
But i think explicit warning is more noticeable for new developer like me.
Regards,
--
Ali Akbar
2014-12-15 11:06 GMT+07:00 Michael Paquier :
>
> On Mon, Dec 15, 2014 at 1:02 PM, Ali Akbar wrote:
> > 2014-12-15 10:19 GMT+07:00 Michael Paquier :
> >>
> >> On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar wrote:
> >> > Peter, while reviewing the better perfo
2014-12-15 11:02 GMT+07:00 Ali Akbar :
>
> 2014-12-15 10:19 GMT+07:00 Michael Paquier :
>>
>> On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar wrote:
>> > Peter, while reviewing the better performing patch myself, now i think
>> the
>> > patch needs more wo
2014-12-15 10:19 GMT+07:00 Michael Paquier :
>
> On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar wrote:
> > Peter, while reviewing the better performing patch myself, now i think
> the
> > patch needs more work to be committed. The structuring of the method
> will be
> >
2014-12-14 22:18 GMT+07:00 Ali Akbar :
>
>
> I ran a test using postgres-US.fo built in the PostgreSQL source tree,
>> which is 38 MB, and ran
>>
>> select unnest(xpath('//fo:bookmark-title', b, array[array['fo',
>> 'http://www.w3.org/199
.energistics.org/schema/prodml_v1.2.0_data/XML_Examples/productVolume_no_xsl.xml
Basically i loaded the xml to table u 100 times. Load script attached.
Regards,
--
Ali Akbar
load_test.sql
Description: application/sql
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2014-11-05 21:50 GMT+07:00 Ali Akbar :
> 2014-11-04 22:16 GMT+07:00 Peter Eisentraut :
>
>> I think the problem this patch is addressing is real, and your approach
>>> is sound, but I'd ask you to go back to the xmlCopyNode() version, and
>>> try to add a test c
2014-11-26 0:38 GMT+07:00 Tom Lane :
> Pavel Stehule writes:
> > 2014-10-27 11:20 GMT+01:00 Ali Akbar :
> >> [ array_agg_anyarray-13.patch ]
>
> > This patch is ready for commit
>
> I've committed this after some significant modifications.
>
> I d
2014-11-04 22:16 GMT+07:00 Peter Eisentraut :
>
> On 10/6/14 10:24 PM, Ali Akbar wrote:
>> > While reviewing the patch myself, i spotted some formatting problems in
>> > the code. Fixed in this v5 patch.
>> >
>> > Also, this patch uses context patch form
2014-11-04 22:16 GMT+07:00 Peter Eisentraut :
> On 10/6/14 10:24 PM, Ali Akbar wrote:
> > While reviewing the patch myself, i spotted some formatting problems in
> > the code. Fixed in this v5 patch.
> >
> > Also, this patch uses context patch format (in first version
ress tests
>
> 6. Patching and compilation is clean without warnings.
>
> This patch is ready for commit
>
> Thank you for patch
>
Thank you for the thorough review process.
Regards,
--
Ali Akbar
ense
>
minor changes in the patch:
* remove array_agg_finalfn_internal declaration in top of array_userfuncs.c
* fix comment of makeMdArray
* fix return of makeMdArray
* remove unnecesary changes to array_agg_finalfn
Regards,
--
Ali Akbar
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.
2014-10-27 9:11 GMT+07:00 Ali Akbar :
>
> 2014-10-27 1:38 GMT+07:00 Pavel Stehule :
>
>> Hi
>>
>> My idea is using new ArrayBuilder optimized for building multidimensional
>> arrays with own State type. I think so casting to ArrayBuildState is base
>> of our
e,
> little bit more complex code is in nodeSubplan.c. Some schematic changes
> are in attachments.
>
Thanks! The structure looks clear, and thanks for the example on
nodeSubplan.c. I will restructure the v10 of the patch to this structure.
Regards,
--
Ali Akbar
NSERT 0 1
> Time: 607,564 ms
> INSERT 0 1
> Time: 610,353 ms
> INSERT 0 1
> Time: 626,816 ms
> INSERT 0 1
> Time: 610,450 ms
> INSERT 0 1
> Time: 614,342 ms
>
Total: 8842,7
> Avg: 631,6
It's 30% faster (i tried varlena element - text). I tried several ti
2014-10-25 15:43 GMT+07:00 Pavel Stehule :
>
>
> 2014-10-25 10:16 GMT+02:00 Ali Akbar :
>
>> makeArrayResult1 - I have no better name now
>>>>
>>>> I found one next minor detail.
>>>>
>>>> you reuse a array_agg_transfn function. I
akeMdArrayResult is used only
in src/pl/plperl/plperl.c:plperl_array_to_datum, while converting perl
array to postgres array.
So if somehow we will accumulate array other than in array_agg, i think the
most natural way is using accumArrayResult and then makeArrayResult.
CMIIW
Regards,
--
Ali Akba
2014-10-25 10:29 GMT+07:00 Ali Akbar :
> I fixed small issue in regress tests and I enhanced tests for varlena
>> types and null values.
>
> Thanks.
>
> it is about 15% faster than original implementation.
>
> 15% faster than array_agg(scalar)? I haven't verify th
-
* array_cat :
* concatenate two nD arrays to form an nD array, or
* push an (n-1)D array onto the end of an nD array
*--------
*/
Regards,
--
Ali Akbar
diff --git a/doc/src/sgml/func.sgml b/d
>= astate->alen)
>^
> arrayfuncs.c:4620:10: error: ‘ArrayBuildState’ has no member named ‘alen’
> astate->alen *= 2;
>
Sorry, correct patch attached.
This patch is in patience format (git --patience ..). In previous patches,
i use context format (git
for array(subselect). patch attached.
Regards,
--
Ali Akbar
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 12046,12051 NULL baz(3 rows)
--- 12046,12067
+ array_agg
+
+array_agg(anyarray)
+
+
+
t (64) there?
>>>>>
>>>>> + astate->abytes = 64 * (ndatabytes == 0 ? 1 : ndatabytes);
>>>>> + astate->aitems = 64 * nitems;
>>>>>
>>>>> + astate->nullbitmap = (bits8 *)
>>>>&g
2014-10-22 22:48 GMT+07:00 Pavel Stehule :
> 2014-10-22 16:58 GMT+02:00 Ali Akbar :
>
>> Thanks for the review
>>
>> 2014-10-22 20:51 GMT+07:00 Pavel Stehule :
>>
>>> I agree with your proposal. I have a few comments to design:
>>>
>>
>
; + repalloc(astate->nullbitmap, (astate->aitems + 7) / 8);
>
just follow the arbitrary size choosen in accumArrayResult (arrayfuncs.c):
astate->alen = 64; /* arbitrary starting array size */
it can be any number not too small and too big. Too small, and we will
realloc shortly. Too big, we will end up wasting memory.
Regards,
--
Ali Akbar
t; - array_agg('{}'::int[]) --> how we determine the dimension of the result?
> is it 0? Or the result will be just an empty array {} ?
>
This updated patch rejects NULL and {} arrays as noted above.
Regards,
--
Ali Akbar
*** a/src/backend/utils/adt/array_userfuncs.c
--
rom http://en.wikipedia.org/wiki/MAC_address,
that there is three numbering namespace for MAC: MAC-48, EUI-48 and EUI-64.
The last one is 64 bits long (8 bytes). Currently PostgreSQL's macaddr is
only 6 bytes long. Should we change it to 8 bytes (not in this patch, of
course)?
Regards,
--
Ali Akbar
o identify asserts (so you're not guessing where the
> assert came from)
>
+1
> - Allows for over-riding individual asserts (so if you need to do
> something you're "not supposed to do" you still have the protection of all
> other asserts)
> - Less verbose than IF THEN RAISE END IF
>
+1
--
Ali Akbar
2014-10-12 19:37 GMT+07:00 Ali Akbar :
> Currently, it cannot handle NULL arrays:
> backend> select array_agg(a) from (values(null::int[])) a(a);
> 1: array_agg(typeid = 1007, len = -1, typmod = -1, byval = f)
>
> ERROR: cannot aggregate null arrays
>
Whi
function.
Currently, it cannot handle NULL arrays:
backend> select array_agg(a) from (values(null::int[])) a(a);
1: array_agg(typeid = 1007, len = -1, typmod = -1, byval = f)
----
ERROR: cannot aggregate null arrays
Regards,
--
Ali Akbar
*** a/src/backend/utils/adt/a
... (cut)
Aside from the sanity check complaints, I don't see any problems in the
resulting array operations.
So, back to the question: Can't we just set the typarray of array types to
its self oid?
Regards,
--
Ali Akbar
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_
case anymore)
Thanks,
Ali Akbar
*** a/src/backend/utils/adt/xml.c
--- b/src/backend/utils/adt/xml.c
***
*** 141,149 static bool print_xml_decl(StringInfo buf, const xmlChar *version,
pg_enc encoding, int standalone);
static xmlDocPtr xml_parse(text *data, XmlOptionType xmloptio
the reimplementation is not as simple as before
(because reimplement some of xmlNodeCopy code must be reimplemented here).
Reviewing the patch myself, i've found some code formatting problems. Will
fix and post in the patch's thread.
Regards,
--
Ali Akbar
ling spaces in the patch that
> should be removed:
>
> +generate_series_numeric(PG_FUNCTION_ARGS)
> ...
> + if (NUMERIC_IS_NAN(start_num) ||
> NUMERIC_IS_NAN(finish_num))
> ...
> + if (PG_NARGS() == 3)
> ...
> + if (NUME
hey should have
the credits for this patch, not me.
In this situation, what should i do?
Thanks for the review Manti!
Regards,
--
Ali Akbar
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 14074,14081 AND
generate_series(start, stop)
!
n my tests, that isn't the case. The code works without
any error and returns the correct rows:
numeric
-
-9
-8
-7
-6
-5
-4
-3
-2
-1
0
1
2
3
4
5
6
7
8
9
(19 rows)
Also, Платон Малюгин, can you add this patch to postgresql commitfest (
http://commitfest.postgresql.org)?
--
Ali Akbar
2014-10-05 15:21 GMT+07:00 Ali Akbar :
> Hi
> Oops, it seems that I have been too hasty here. With a fresh mind I looked
> at my own patch again and found two bugs:
>
>
>>> - Incorrect calculation of each step's value, making stuff crash, it is
>>> nec
131072 digits before the decimal point; up to 16383
digits after the decimal point. How can we check if the next step
overflows? As a comparison, in int.c, generate_series_step_int4 checks if
its overflows, and stop the next call by setting step to 0. Should we do
that?
~ will try to fix the patch later
--
Ali Akbar
to fix it (which needs updates to xml_1.out, which I omit here for
> simplicity).
>
The patch works, albeit must be applied with --ignore-whitespace. Attached
the patch + xml_1.out updates.
I'm marking this ready for commit
--
Ali Akbar
diff --git a/src/backend/utils/adt/xml.c b/src/ba
node could be dumped again in next xpath result).
Thanks,
Ali Akbar
2014-07-11 15:36 GMT+07:00 Ali Akbar :
> Greetings,
>
> Attached modified patch to handle xmlCopyNode returning NULL. The patch is
> larger because xmlerrcxt must be passed to xml_xmlnodetoxmltype (xmlerrcxt
>
doesn't
check the return of xmlStaticCopyNode whether it's NULL. So if someday the
recursion returns NULL (maybe because of memory exhaustion), it will
SEGFAULT.
Knowing this but in libxml2 code, is this patch is still acceptable?
Thanks,
Ali Akbar
*** a/src/backend/utils/adt/xml.c
--- b
nt for.
>
Ah, overlooked that.
Skimming through libxml2's source, it looks like there are some other cases
beside out-of-memory. Will update the patch according to these cases.
Thanks for the review.
--
Ali Akbar
>
> I'm marking this ready for committer.
>
Thanks for the review. Hope i will be able to contribute a little here and
there in the future.
--
Ali Akbar
your patch doesn't perform better, it's more easier to just use
crosstab. For storing it efficiently, the result can be transformed
into array manually.
PS: as Michael Paquier said above, its better if you could send the
patch in the .patch file format (see:
https://wiki.postgresql.org/wiki/
unt() aggregates, but it produces array. If the values are not
sparse, i think the performance and memory/storage benefit you
mentioned will be true. But if the values are sparse, there will be
many 0's, how it will perform?
I'm interested to benchmark it with some use cases, to confirm th
s, i think because this patch changes xpath() behavior, we
will only apply this to future versions. The old behavior is wrong
(according to XPath standard) for not including namespaces, but maybe there
are some application that depends on the old behavior.
Thanks!
--
Ali Akbar
or
its children). When the copy dumped, the resulting XML is complete with its
namespaces. Calling xmlCopyNode will need additional memory to execute, but
reimplementing its routine to handle namespace definition will introduce
much complexity to the code.
Note: This is my very first postgresql
61 matches
Mail list logo
