Horton

Thread

Commits

  1. Remove unnecessary #include.

  2. Refactor planner's header files.

  3. Support named and default arguments in CALL

  1. optimizer/clauses.h needn't include access/htup.h

    Justin Pryzby <pryzby@telsasoft.com> — 2020-11-23T20:55:05Z

    It was only needed between these:

commit a8677e3ff6bb8ef78a9ba676faa647bba237b1c4
Author: Peter Eisentraut <peter_e@gmx.net>
Date:   Fri Apr 13 17:06:28 2018 -0400

    Support named and default arguments in CALL

commit f09346a9c6218dd239fdf3a79a729716c0d305bd
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Tue Jan 29 15:48:51 2019 -0500

    Refactor planner's header files.

I noticed while looking at "what includes what" and wondered if some of these
are kind of "modularity violations". 

$ find src/include/ -name '*.h' -print0 |xargs -r0 awk -F'[/"]' 'NF>2 && /^#include "/{split(FILENAME,a); print a[3],"-",$2 }' |awk '$1!=$3 && !/\.h|statistics|partitioning|bootstrap|tsearch|foreign|jit|regex|lib|common/' |sort |uniq -c |sort -nr |awk '$1==1'
      1 utils - rewrite
      1 utils - port
      1 utils - parser
      1 tcop - storage
      1 tcop - executor
      1 tcop - catalog
      1 tcop - access
      1 storage - postmaster
      1 rewrite - catalog
      1 rewrite - access
      1 replication - port
      1 replication - datatype
      1 postmaster - datatype
      1 parser - catalog
      1 nodes - commands
      1 executor - portability
      1 executor - port
      1 commands - datatype
      1 catalog - port
      1 catalog - parser
      1 access - tcop
      1 access - replication
      1 access - postmaster
      1 access - executor

pryzbyj@pryzbyj:~/src/postgres$ find src/backend/ -name '*.c' -print0 |xargs -r0 awk -F'[/"]' 'NF>2 && /^#include "/{split(FILENAME,a); print a[3],"-",$2 }' |awk '$1!=$3 && !/\.h/&&!/common|utils|tsearch|main|foreign|port|regex|bootstrap|jit/' |sort |uniq -c |sort -nr |awk '$1==1'
      1 storage - libpq
      1 statistics - postmaster
      1 statistics - commands
      1 rewrite - tcop
      1 rewrite - optimizer
      1 replication - syncrep_scanner.c
      1 replication - rewrite
      1 replication - repl_scanner.c
      1 replication - optimizer
      1 postmaster - mb
      1 partitioning - rewrite
      1 partitioning - commands
      1 nodes - mb
      1 libpq - postmaster
      1 libpq - mb
      1 libpq - commands

-- 
Justin

  2. Re: optimizer/clauses.h needn't include access/htup.h

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-11-23T22:00:41Z

    Justin Pryzby <pryzby@telsasoft.com> writes:
> It was only needed between these:
> commit a8677e3ff6bb8ef78a9ba676faa647bba237b1c4
> commit f09346a9c6218dd239fdf3a79a729716c0d305bd

Hm, you're right.  Removed.

> I noticed while looking at "what includes what" and wondered if some of these
> are kind of "modularity violations". 

Yeah.  I've ranted before that we ought to have some clearer idea of
module layering within the backend, and avoid cross-header inclusions
that would break the layering.  This particular case didn't really
do so, I suppose, since htup.h would surely be on a lower level than
the optimizer.  But it still seems nicer to not have that inclusion.

Anyway, if you're feeling motivated to explore a more wide-ranging
refactoring, by all means have a go at it.

			regards, tom lane

  3. Re: optimizer/clauses.h needn't include access/htup.h

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2020-11-23T22:44:37Z

    On 2020-Nov-23, Tom Lane wrote:

> Anyway, if you're feeling motivated to explore a more wide-ranging
> refactoring, by all means have a go at it.

I was contemplating commands/trigger.c this morning (after Heikki split
copy.c) thinking about the three pieces embedded in there -- one
catalog/pg_trigger.c, one in executor (execTrigger.c?) and what seems a
very small piece to remain where it is.

Just thinking out loud ...

  4. Re: optimizer/clauses.h needn't include access/htup.h

    Andres Freund <andres@anarazel.de> — 2020-12-20T21:38:41Z

    Hi,

On 2020-11-23 19:44:37 -0300, Alvaro Herrera wrote:
> I was contemplating commands/trigger.c this morning (after Heikki split
> copy.c) thinking about the three pieces embedded in there -- one
> catalog/pg_trigger.c, one in executor (execTrigger.c?) and what seems a
> very small piece to remain where it is.

One thing that's not clear enough in general - at least in my view - is
what belongs into executor/ and what should be somewhere more
general. E.g. a lot of what I assume you would move to execTrigger.c is
not just used within the real executor, but also from e.g. copy. There's
plenty pre-existing examples for that (e.g. tuple slots), but I wonder
if we should try to come up with a better split at some point.


Oh, and definitely +1 on splitting trigger.c. Wonder if the the trigger
queue stuff, and the directly executor interfacing functions should be
split again? It seems to me the trigger queue details are isolated
enough that that could come out clean enough.

- Andres