Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

Noah Misch writes: > What, if anything, is yet required to close this 9.6 open item? The original complaint about polymorphic aggs is fixed to my satisfaction. The business about how non-text-format EXPLAIN reports parallel plans (<16002.1466972...@sss.pgh.pa.us>) remains, but

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On Thu, Jun 23, 2016 at 10:57:26AM -0400, Tom Lane wrote: > David Rowley writes: > > On 23 June 2016 at 11:22, Tom Lane wrote: > >> The behavior that I'd expect (and that I documented) for a deserialization > >> function is that it just allocates

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

David Rowley writes: > On 23 June 2016 at 11:22, Tom Lane wrote: >> The behavior that I'd expect (and that I documented) for a deserialization >> function is that it just allocates its result in the current, short-lived >> memory context, since

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On 23 June 2016 at 11:22, Tom Lane wrote: > While working on that, I noticed what seems to me to be a minor bug. > The behavior that I'd expect (and that I documented) for a deserialization > function is that it just allocates its result in the current, short-lived > memory

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

David Rowley writes: > I've attached my proposed patch for the user defined aggregate docs. I whacked this around some more and pushed it. While working on that, I noticed what seems to me to be a minor bug. The behavior that I'd expect (and that I documented) for

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On 23 June 2016 at 08:53, Tom Lane wrote: > I wrote: >> David Rowley writes: >>> I've gone and implemented the dummy argument approach for >>> deserialization functions. > >> How do you feel about the further idea of locking down the signatures

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

I wrote: > David Rowley writes: >> I've gone and implemented the dummy argument approach for >> deserialization functions. > How do you feel about the further idea of locking down the signatures > to be exactly "serialize(internal) returns bytea" and

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

David Rowley writes: > I've gone and implemented the dummy argument approach for > deserialization functions. How do you feel about the further idea of locking down the signatures to be exactly "serialize(internal) returns bytea" and "deserialize(bytea, internal)

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On 20 June 2016 at 22:19, David Rowley wrote: > I've gone and implemented the dummy argument approach for > deserialization functions. > > If we go with this, I can then write the docs for 35.10 which'll serve > to explain parallel user defined aggregates in detail.

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On 20 June 2016 at 19:06, David Rowley wrote: > On 18 June 2016 at 05:45, Tom Lane wrote: >> A possible solution is to give deserialize an extra dummy argument, along >> the lines of "deserialize(bytea, internal) returns internal", thereby >>

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On 18 June 2016 at 09:29, Tom Lane wrote: > So at this point my proposal is: > > 1. Add an OID-list field to Aggref holding the data types of the > user-supplied arguments. This can be filled in parse analysis since it > won't change thereafter. Replace calls to

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On 18 June 2016 at 05:45, Tom Lane wrote: > The situation is much direr as far as serialize/deserialize are concerned. > These basically don't work at all for polymorphic transtypes: if you try > to declare "deserialize(bytea) returns anyarray", the type system won't > let

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

I wrote: > Meanwhile, I'll go see if I can work out exactly what's broken about the > polymorphic type-slinging. That I would like to see working in beta2. OK, I've got at least the core of the problem. fix_combine_agg_expr changes the upper aggregate's Aggref node so that its args list is a

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

Tom Lane wrote: > Alvaro Herrera writes: > > Can't we have them be ExtensibleNode? > > No, these are data types and values of data types, not parsetree nodes. Ah, right. > The core problem is to teach the type system to prevent you from > sticking foo(internal) into

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

Alvaro Herrera writes: > Tom Lane wrote: >> Robert Haas writes: >>> I think we should break up internal into various kinds of internal >>> depending on what kind of a thing we've actually got a pointer to. >> Not a bad long-term project, but it's

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On Fri, Jun 17, 2016 at 3:14 PM, Tom Lane wrote: > The concern I have is that you could stick it into an aggregate that isn't > the one it's expecting to be used in, or into a slot in that aggregate > that isn't deserialize(), and the run-time test can't detect either of >

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

Tom Lane wrote: > Robert Haas writes: > > I think we should break up internal into various kinds of internal > > depending on what kind of a thing we've actually got a pointer to. > > Not a bad long-term project, but it's not happening in this cycle. > I'm not very sure

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

Robert Haas writes: > On Fri, Jun 17, 2016 at 1:45 PM, Tom Lane wrote: >> there's a far bigger problem which is that "deserialize(bytea) returns >> internal" is a blatant security hole, which I see you ignored the defense >> against in opr_sanity.sql.

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On Fri, Jun 17, 2016 at 1:45 PM, Tom Lane wrote: > I wrote: >> Robert Haas writes: >>> I don't mind if you feel the need to refactor this. > >> I'm not sure yet. What I think we need to work out first is exactly >> how polymorphic parallel aggregates

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

I wrote: > Robert Haas writes: >> I don't mind if you feel the need to refactor this. > I'm not sure yet. What I think we need to work out first is exactly > how polymorphic parallel aggregates ought to identify which concrete > data types they are using. There were

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

Robert Haas writes: > On Thu, Jun 16, 2016 at 6:40 PM, Tom Lane wrote: >> Meh. I think this is probably telling us that trying to repurpose Aggref >> as the representation of a partial aggregate may have been mistaken. > I don't mind if you feel the

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On Thu, Jun 16, 2016 at 6:40 PM, Tom Lane wrote: > Meh. I think this is probably telling us that trying to repurpose Aggref > as the representation of a partial aggregate may have been mistaken. > Or maybe just that fix_combine_agg_expr was a bad idea. It seems likely > to

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

Robert Haas writes: > On Thu, Jun 16, 2016 at 4:31 PM, Tom Lane wrote: >> However, before trying to fix any of that, I'd like to demand an >> explanation as to why aggoutputtype exists at all. It seems incredibly >> confusing to have both that and

Re: [HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

On Thu, Jun 16, 2016 at 4:31 PM, Tom Lane wrote: > However, before trying to fix any of that, I'd like to demand an > explanation as to why aggoutputtype exists at all. It seems incredibly > confusing to have both that and aggtype, and certainly the code comments > give no

[HACKERS] Parallelized polymorphic aggs, and aggtype vs aggoutputtype

I believe I've narrowed down the cause of this failure [1]: regression=# explain SELECT min(col) FROM enumtest; ERROR: type matched to anyenum is not an enum type: anyenum The core reason for this seems to be that apply_partialaggref_adjustment fails to consider the possibility that it's