Re: reorganizing partitioning code - Mailing list pgsql-hackers
From | Amit Langote |
---|---|
Subject | Re: reorganizing partitioning code |
Date | |
Msg-id | ab44c185-0977-c1fe-0fec-813cc187c5d8@lab.ntt.co.jp Whole thread Raw |
In response to | Re: reorganizing partitioning code (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>) |
Responses |
Re: Re: reorganizing partitioning code
|
List | pgsql-hackers |
Horiguchi-san, On 2018/02/16 14:07, Kyotaro HORIGUCHI wrote: > Hello. I'd like to make a humble comment. Thanks a lot for taking a look. > The reorganization adds/removes header files to/from .[ch] > files. That can easily leave useless includes and they're hardly > noticed. Following files touched in this patch have such useless > includes after this patch. Yes, I agree. > On the other hand, naive decision of this kind of cleanup can > lead to curruption. [1] So, I don't insisit that the all of the > following *should* amended, especially for rel.h. > > [1] https://www.postgresql.org/message-id/6748.1518711125@sss.pgh.pa.us I was initially trying limit this patch's #include churn so that it only touches .[ch] files that are somehow involved with partitioning, but ended up with a lot of files touched. To prevent confusion and concerns, I have updated the patch to keep the churn to minimum as best as I could. So, while the v3 patch looked like this: 24 files changed, 2373 insertions(+), 2312 deletions(-) create mode 100644 src/backend/utils/cache/partcache.c create mode 100644 src/include/utils/partcache.h v4 looks like this: 15 files changed, 2314 insertions(+), 2263 deletions(-) create mode 100644 src/backend/utils/cache/partcache.c create mode 100644 src/include/utils/partcache.h > ==== nodeModifyTable.c: >> +#include "rewrite/rewriteManip.h" > > rewriteManip.h is changed to include rel.h so rel.h is no longer > need to be included in the file. (It is also included in lmgr.h > so it was needless since before this patch, though.) On second thought, I decided to hold back on moving map_partition_varattnos() to rewriteManip.c, because its interface is unlike any other functions in that file, requiring us to include rel.h in rewriteManip.h. So, I removed this #include from nodeModifyTable.c. > ==== hba.c: >> +#include "catalog/objectaddress.h" > > objectaddress.h includes acl.h so acl.h is no longer useful. This and a few others were necessitated by removing partition.h from executor.h. For now, I'm putting partition.h back into executor.h, although it would be nice to remove it eventually. > ==== joinrels.c: >> +#include "utils/partcache.h" > > partcache.h includes lsyscache.h. Moved lsyscache.h from partcache.h to partcache.c. > ==== prepunion.c: >> +#include "utils/partcache.h" > > partcache.h includes lsyscache.h and partcache.h is included in > rel.h. So partcache.h and lsyscache.h are not required. Oops, why did I even include partcache.h in prepunion.c! Removed. > ==== utility.c: >> +#include "utils/rel.h" > > rel.h includes xlog.h (and xlog.h includes fd.h). The last two > are removable. I've reverted the change that necessitated this and so this one. > ==== partcache.c: > parsenodes.h is included from some other files. > rel.h is included from rewriteManip.h. > partcache.h is included from rel.h > As the result, parsenodes.h, rel.h, partcache.h are not required. Removed. > ==== relcache.c: >> +#include "utils/partcache.h" > > lsyscache.h is included by partcache.h. lsyscache.h was moved from partcache.h per above, so keeping here. > ==== rel.h: >> +#include "utils/partcache.h" > > partcache.h includes fmgr.h and relcache.h so the last two are > no longer useful. Removed both. Attached updated version. Thanks, Amit
Attachment
pgsql-hackers by date: