On Mon, Feb 6, 2017 at 11:10 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> writes: >> On Mon, Feb 6, 2017 at 10:34 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> I'd vote for not. The general programming style in the backend is to >>> ignore the fact that makeNode() zeroes the node's storage and initialize >>> all the fields explicitly. The primary advantage of that, IMO, is that >>> you can grep for references to a particular field and understand >>> everyplace that affects it. As an example of the value, if you want to >>> add a field X and you try to figure out what you need to modify by >>> grepping for references to existing field Y, with this proposal you would >>> miss places that were node initializations unless Y *always* needed to be >>> initialized nonzero. > >> In the case of adding a new field, I would rather rely on where the >> structure itself is used rather than another member. > > Maybe. There's certainly something to be said for developing a different > style in which we only initialize fields that need to be nonzero ... but > if we were to go for that, relnode.c would not be the only place to fix, > or even the best place to start. >
As against any other Node structure, RelOptInfo somehow stood out clearly for me in this aspect. May be because it's a large structure. But yes, this might be a problem with other structures as well. > Also, grepping for makeNode(FooStruct) works for cases where FooStructs > are *only* built that way. But there's more than one place in the backend > where we build structs in other ways, typically by declaring one as a > local variable. It would take some effort to define a universal > convention here and make sure all existing code follows it. > I think only makeNode() or palloc0(... * sizeof(nodename)) usages can benefit from this. In other cases the fields need to be explicitly initialized. Also, that would be a lot of code churn. May be we should restrict to some large Node structures like RelOptInfo. >> Should we at least move those assignments into a separate function? > > As far as relnode.c goes, I don't really think that's an improvement, > because it complicates dealing with fields that need to be initialized > differently for baserels and joinrels. > I am thinking more on the lines of makeVar() - create a function makeRelOptInfo() or such, which accepts values for fields which need assignment other than zero value and call it from build_join_rel() and build_simple_rel() passing joinrel and base rel specific values resp. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
