Hi, While working on pluggable storage (in particular, while cleaning it up over the last few days), I grew concerned with widely heapam.h is included in other headers. E.g. the executor (via execnodes.h, executor.h relying on heapam.h) shouldn't depend on heapam.h details - particularly after pluggable storage, but also before. To me that's unnecessary leakage across abstraction boundaries.
In the attached patch I excised all heapam.h includes from other headers. There were basically two things required to do so: * In a few places that use HeapScanDesc (which confusingly is a typedef in heapam.h, but the underlying struct is in relscan.h) etc, we can easily get by just using struct HeapScanDescData * instead. * I moved the LockTupleMode enum to to lockoptions.h - previously executor.h tried to avoid relying on heapam.h, but it ended up including heapam.h indirectly, which lead to a couple people introducing new uses of the enum. We could just convert those to ints like in other places, but I think moving to a smaller header seems more appropriate. I don't think lockoptions.h is perfect, but it's also not bad? This required adding heapam.h includes to a bunch of places, but that doesn't seem too bad. It'll possibly break a few external users, but I think that's acceptable cost - many of those will already/will further be broken in 12 anyway. I think we should do the same with genam.h, but that seems better done separately. I've a pending local set of patches that splits relation_open/close, heap_open/close et al into a separate set of includes, that then allows to downgrade the heapam.h include to that new file (given that a large percentage of the files really just want heap_open/close and nothing else from heapam.h), which I'll rebase ontop of this if we can agree that this change is a good idea. Alvaro, you'd introduced the split of HeapScanDesc and HeapScanDescData being in different files (in a3540b0f657c6352) - what do you think about this change? I didn't revert that split, but I think we ought to consider just relying on a forward declared struct in heapam.h instead, this split is pretty confusing and seems to lead to more interdependence in a lot of cases. Greetings, Andres Freund
>From 60dafe4dd1f112c506c5c6abd7b9fe58a2517473 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Sun, 13 Jan 2019 15:31:58 -0800 Subject: [PATCH] Don't include heapam.h from others headers. heapam.h previously was included in a number of widely used headers (e.g. execnodes.h, indirectly in executor.h, ...). That's problematic on its own, as heapam.h contains a lot of low-level details that don't need to be exposed that widely, but becomes more problematic with the upcoming introduction of pluggable table storage - it seems inappropriate for heapam.h to be included that widely afterwards. heapam.h was largely only included in other headers to get the HeapScanDesc typedef (which was defined in heapam.h, even though HeapScanDescData is defined in relscan.h). The better solution here seems to be to just use the underlying struct (forward declared where necessary). Similar for BulkInsertState. Another problem was that LockTupleMode was used in executor.h - parts of the file tried to cope without heapam.h, but due to the fact that it indirectly did so already, several violations of that goal were not not noticed. We could just reuse the approach of declaring parameters as int, but it seems nicer to move LockTupleMode to lockoptions.h - that's not perfect, but also doesn't seem bad. As a number of files relied on implicitly included heapam.h, a significant number of files grew an explicit include. It's quite probably that a few external projects will need to do the same. Author: Andres Freund Discussion: https://postgr.es/m/ --- contrib/amcheck/verify_nbtree.c | 1 + contrib/dblink/dblink.c | 1 + contrib/file_fdw/file_fdw.c | 1 + contrib/pageinspect/btreefuncs.c | 1 + contrib/pageinspect/heapfuncs.c | 1 + contrib/pageinspect/rawpage.c | 1 + contrib/pg_freespacemap/pg_freespacemap.c | 1 + contrib/pg_visibility/pg_visibility.c | 1 + contrib/pgrowlocks/pgrowlocks.c | 1 + contrib/pgstattuple/pgstatapprox.c | 1 + contrib/pgstattuple/pgstattuple.c | 1 + contrib/postgres_fdw/postgres_fdw.c | 1 + contrib/tsm_system_rows/tsm_system_rows.c | 1 + contrib/tsm_system_time/tsm_system_time.c | 1 + src/backend/access/brin/brin.c | 1 + src/backend/access/common/indextuple.c | 2 +- src/backend/access/index/genam.c | 1 + src/backend/access/index/indexam.c | 1 + src/backend/access/nbtree/nbtsort.c | 1 + src/backend/access/tablesample/system.c | 1 + src/backend/bootstrap/bootstrap.c | 1 + src/backend/catalog/dependency.c | 1 + src/backend/catalog/heap.c | 1 + src/backend/catalog/index.c | 1 + src/backend/catalog/indexing.c | 1 + src/backend/catalog/objectaddress.c | 1 + src/backend/catalog/pg_proc.c | 1 + src/backend/catalog/toasting.c | 1 + src/backend/commands/alter.c | 1 + src/backend/commands/analyze.c | 1 + src/backend/commands/cluster.c | 1 + src/backend/commands/constraint.c | 1 + src/backend/commands/createas.c | 1 + src/backend/commands/event_trigger.c | 1 + src/backend/commands/extension.c | 1 + src/backend/commands/indexcmds.c | 1 + src/backend/commands/matview.c | 1 + src/backend/commands/sequence.c | 1 + src/backend/commands/statscmds.c | 1 + src/backend/commands/typecmds.c | 1 + src/backend/executor/execMain.c | 11 +++-------- src/backend/executor/execPartition.c | 1 + src/backend/executor/execReplication.c | 1 + src/backend/executor/execUtils.c | 1 + src/backend/executor/nodeBitmapHeapscan.c | 1 + src/backend/executor/nodeLockRows.c | 1 + src/backend/executor/nodeModifyTable.c | 1 + src/backend/executor/nodeSamplescan.c | 1 + src/backend/executor/nodeSeqscan.c | 1 + src/backend/executor/nodeTidscan.c | 1 + src/backend/optimizer/plan/planner.c | 1 + src/backend/parser/parse_relation.c | 1 + src/backend/parser/parse_utilcmd.c | 1 + src/backend/partitioning/partbounds.c | 1 + src/backend/replication/logical/tablesync.c | 1 + src/backend/replication/logical/worker.c | 1 + src/backend/rewrite/rewriteHandler.c | 1 + src/backend/utils/adt/misc.c | 1 + src/backend/utils/adt/ri_triggers.c | 1 + src/backend/utils/adt/ruleutils.c | 1 + src/backend/utils/adt/selfuncs.c | 1 + src/backend/utils/adt/xml.c | 1 + src/backend/utils/cache/relcache.c | 1 + src/backend/utils/fmgr/funcapi.c | 1 + src/include/access/heapam.h | 15 --------------- src/include/access/hio.h | 3 +-- src/include/access/relscan.h | 5 ++--- src/include/catalog/index.h | 4 ++-- src/include/executor/executor.h | 9 +++++---- src/include/nodes/execnodes.h | 3 +-- src/include/nodes/lockoptions.h | 15 +++++++++++++++ 71 files changed, 92 insertions(+), 37 deletions(-) diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index a8a0ec70e1a..055bfa05707 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -23,6 +23,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/nbtree.h" #include "access/transam.h" diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index bf2a6c343d3..203f6fd3f01 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -38,6 +38,7 @@ #include "access/htup_details.h" #include "access/reloptions.h" +#include "access/heapam.h" #include "catalog/indexing.h" #include "catalog/namespace.h" #include "catalog/pg_foreign_data_wrapper.h" diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index 9ff34b4357f..ad4da6b5ed4 100644 --- a/contrib/file_fdw/file_fdw.c +++ b/contrib/file_fdw/file_fdw.c @@ -18,6 +18,7 @@ #include "access/htup_details.h" #include "access/reloptions.h" #include "access/sysattr.h" +#include "access/heapam.h" #include "catalog/pg_authid.h" #include "catalog/pg_foreign_table.h" #include "commands/copy.h" diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index 184ac62255c..454e488f86a 100644 --- a/contrib/pageinspect/btreefuncs.c +++ b/contrib/pageinspect/btreefuncs.c @@ -30,6 +30,7 @@ #include "pageinspect.h" #include "access/nbtree.h" +#include "access/heapam.h" #include "catalog/namespace.h" #include "catalog/pg_am.h" #include "funcapi.h" diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c index a0c703750a5..b98c4881ac5 100644 --- a/contrib/pageinspect/heapfuncs.c +++ b/contrib/pageinspect/heapfuncs.c @@ -27,6 +27,7 @@ #include "pageinspect.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "funcapi.h" #include "catalog/pg_type.h" diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c index abd17d331bd..2d91954cb7d 100644 --- a/contrib/pageinspect/rawpage.c +++ b/contrib/pageinspect/rawpage.c @@ -18,6 +18,7 @@ #include "pageinspect.h" #include "access/htup_details.h" +#include "access/heapam.h" #include "catalog/namespace.h" #include "catalog/pg_type.h" #include "funcapi.h" diff --git a/contrib/pg_freespacemap/pg_freespacemap.c b/contrib/pg_freespacemap/pg_freespacemap.c index 7d939a7d207..5ad68005573 100644 --- a/contrib/pg_freespacemap/pg_freespacemap.c +++ b/contrib/pg_freespacemap/pg_freespacemap.c @@ -8,6 +8,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "funcapi.h" #include "storage/freespace.h" diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c index 210224e8e14..22adca18e64 100644 --- a/contrib/pg_visibility/pg_visibility.c +++ b/contrib/pg_visibility/pg_visibility.c @@ -10,6 +10,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/visibilitymap.h" #include "catalog/pg_type.h" diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c index 94e051d642b..fffee6e27e0 100644 --- a/contrib/pgrowlocks/pgrowlocks.c +++ b/contrib/pgrowlocks/pgrowlocks.c @@ -24,6 +24,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/multixact.h" #include "access/relscan.h" #include "access/xact.h" diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c index ca8ccc360d1..9729d8eb1bd 100644 --- a/contrib/pgstattuple/pgstatapprox.c +++ b/contrib/pgstattuple/pgstatapprox.c @@ -13,6 +13,7 @@ #include "postgres.h" #include "access/visibilitymap.h" +#include "access/heapam.h" #include "access/transam.h" #include "access/xact.h" #include "access/multixact.h" diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c index 6d67bd8271c..8f67c3c2209 100644 --- a/contrib/pgstattuple/pgstattuple.c +++ b/contrib/pgstattuple/pgstattuple.c @@ -26,6 +26,7 @@ #include "access/gist_private.h" #include "access/hash.h" +#include "access/heapam.h" #include "access/nbtree.h" #include "access/relscan.h" #include "catalog/namespace.h" diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index e0c68dc6b4c..64efbdff082 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -14,6 +14,7 @@ #include "postgres_fdw.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" #include "catalog/pg_class.h" diff --git a/contrib/tsm_system_rows/tsm_system_rows.c b/contrib/tsm_system_rows/tsm_system_rows.c index 2dc14461cdc..69944f61c8d 100644 --- a/contrib/tsm_system_rows/tsm_system_rows.c +++ b/contrib/tsm_system_rows/tsm_system_rows.c @@ -28,6 +28,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/relscan.h" #include "access/tsmapi.h" #include "catalog/pg_type.h" diff --git a/contrib/tsm_system_time/tsm_system_time.c b/contrib/tsm_system_time/tsm_system_time.c index 50c3db06428..f516552cc02 100644 --- a/contrib/tsm_system_time/tsm_system_time.c +++ b/contrib/tsm_system_time/tsm_system_time.c @@ -26,6 +26,7 @@ #include <math.h> +#include "access/heapam.h" #include "access/relscan.h" #include "access/tsmapi.h" #include "catalog/pg_type.h" diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 03c8a6b3ce7..52144271c1c 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -19,6 +19,7 @@ #include "access/brin_page.h" #include "access/brin_pageops.h" #include "access/brin_xlog.h" +#include "access/heapam.h" #include "access/reloptions.h" #include "access/relscan.h" #include "access/xloginsert.h" diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c index bc0c614f3b7..6a22b172036 100644 --- a/src/backend/access/common/indextuple.c +++ b/src/backend/access/common/indextuple.c @@ -16,7 +16,7 @@ #include "postgres.h" -#include "access/heapam.h" +#include "access/htup_details.h" #include "access/itup.h" #include "access/tuptoaster.h" diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c index 4d46257d6a0..e632ad0a9bc 100644 --- a/src/backend/access/index/genam.c +++ b/src/backend/access/index/genam.c @@ -19,6 +19,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/relscan.h" #include "access/transam.h" #include "catalog/index.h" diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index 4a0238f9edc..676355501bf 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -70,6 +70,7 @@ #include "postgres.h" #include "access/amapi.h" +#include "access/heapam.h" #include "access/relscan.h" #include "access/transam.h" #include "access/xlog.h" diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c index d9b9229ab76..83966dc4d41 100644 --- a/src/backend/access/nbtree/nbtsort.c +++ b/src/backend/access/nbtree/nbtsort.c @@ -57,6 +57,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/nbtree.h" #include "access/parallel.h" #include "access/relscan.h" diff --git a/src/backend/access/tablesample/system.c b/src/backend/access/tablesample/system.c index e9d5e6d2221..383387bc695 100644 --- a/src/backend/access/tablesample/system.c +++ b/src/backend/access/tablesample/system.c @@ -27,6 +27,7 @@ #include <math.h> #include "access/hash.h" +#include "access/heapam.h" #include "access/relscan.h" #include "access/tsmapi.h" #include "catalog/pg_type.h" diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 5274e26783e..776533a2efb 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -17,6 +17,7 @@ #include <unistd.h> #include <signal.h> +#include "access/heapam.h" #include "access/htup_details.h" #include "access/xact.h" #include "access/xlog_internal.h" diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index dc679ed8b99..f5560e6f706 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -15,6 +15,7 @@ #include "postgres.h" #include "access/htup_details.h" +#include "access/heapam.h" #include "access/xact.h" #include "catalog/dependency.h" #include "catalog/heap.h" diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 472285d3913..149d97e3c3b 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -29,6 +29,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/multixact.h" #include "access/sysattr.h" diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index c91408046af..761218f0574 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -24,6 +24,7 @@ #include <unistd.h> #include "access/amapi.h" +#include "access/heapam.h" #include "access/multixact.h" #include "access/relscan.h" #include "access/reloptions.h" diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c index 20ae5a77189..954e3f9ab65 100644 --- a/src/backend/catalog/indexing.c +++ b/src/backend/catalog/indexing.c @@ -15,6 +15,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "catalog/index.h" #include "catalog/indexing.h" diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 0ad8e46b105..66c2f54d5bf 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -15,6 +15,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" #include "catalog/catalog.h" diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 88d6f5d0f98..8c2e42d7590 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -15,6 +15,7 @@ #include "postgres.h" #include "access/htup_details.h" +#include "access/heapam.h" #include "access/xact.h" #include "catalog/catalog.h" #include "catalog/dependency.h" diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index 052a0a13051..827e1c82dd6 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -14,6 +14,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/tuptoaster.h" #include "access/xact.h" #include "catalog/binary_upgrade.h" diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index 0217c537a77..3c53178f75c 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -14,6 +14,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" #include "catalog/dependency.h" diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 9581c2c0287..4248def61a7 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -16,6 +16,7 @@ #include <math.h> +#include "access/heapam.h" #include "access/multixact.h" #include "access/sysattr.h" #include "access/transam.h" diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 32f55cda570..a89210808b3 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -18,6 +18,7 @@ #include "postgres.h" #include "access/amapi.h" +#include "access/heapam.h" #include "access/multixact.h" #include "access/relscan.h" #include "access/rewriteheap.h" diff --git a/src/backend/commands/constraint.c b/src/backend/commands/constraint.c index 7fc3bcad89b..b9aec7d18be 100644 --- a/src/backend/commands/constraint.c +++ b/src/backend/commands/constraint.c @@ -13,6 +13,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "catalog/index.h" #include "commands/trigger.h" #include "executor/executor.h" diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index 7185432763f..5947996d673 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -24,6 +24,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/reloptions.h" #include "access/htup_details.h" #include "access/sysattr.h" diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index e61c84ce721..e5083349814 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -13,6 +13,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/xact.h" #include "catalog/catalog.h" diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 1f85f223a0a..f63238454a3 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -29,6 +29,7 @@ #include <sys/stat.h> #include <unistd.h> +#include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" #include "access/xact.h" diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index d2639036223..62bc309e784 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -16,6 +16,7 @@ #include "postgres.h" #include "access/amapi.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/reloptions.h" #include "access/sysattr.h" diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index c322d008886..846bd7608ce 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -14,6 +14,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/multixact.h" #include "access/xact.h" diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index 46e9c91ede7..1df2df8ff55 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -15,6 +15,7 @@ #include "postgres.h" #include "access/bufmask.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/multixact.h" #include "access/transam.h" diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index ad385785daa..86a96ac05dc 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -14,6 +14,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/relscan.h" #include "catalog/catalog.h" #include "catalog/dependency.h" diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 22b0d5d47e8..769504c4591 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -31,6 +31,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/xact.h" #include "catalog/binary_upgrade.h" diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 26e41902f3a..ed0330056bf 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -37,6 +37,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" #include "access/transam.h" @@ -2435,13 +2436,10 @@ ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist) * * Returns a slot containing the new candidate update/delete tuple, or * NULL if we determine we shouldn't process the row. - * - * Note: properly, lockmode should be declared as enum LockTupleMode, - * but we use "int" to avoid having to include heapam.h in executor.h. */ TupleTableSlot * EvalPlanQual(EState *estate, EPQState *epqstate, - Relation relation, Index rti, int lockmode, + Relation relation, Index rti, LockTupleMode lockmode, ItemPointer tid, TransactionId priorXmax) { TupleTableSlot *slot; @@ -2522,12 +2520,9 @@ EvalPlanQual(EState *estate, EPQState *epqstate, * * If successful, we have locked the newest tuple version, so caller does not * need to worry about it changing anymore. - * - * Note: properly, lockmode should be declared as enum LockTupleMode, - * but we use "int" to avoid having to include heapam.h in executor.h. */ HeapTuple -EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, +EvalPlanQualFetch(EState *estate, Relation relation, LockTupleMode lockmode, LockWaitPolicy wait_policy, ItemPointer tid, TransactionId priorXmax) { diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 3a1004ff477..7415dfa45eb 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -13,6 +13,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "catalog/partition.h" #include "catalog/pg_inherits.h" #include "catalog/pg_type.h" diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index e9c1beb1b76..6eca2f5c2d6 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -14,6 +14,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/relscan.h" #include "access/transam.h" #include "access/xact.h" diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 24ab43d5e5c..d914d4b5006 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -45,6 +45,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/parallel.h" #include "access/relscan.h" #include "access/transam.h" diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index f8d70f25920..cd20abc141e 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -37,6 +37,7 @@ #include <math.h> +#include "access/heapam.h" #include "access/relscan.h" #include "access/transam.h" #include "access/visibilitymap.h" diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index c80536931ec..6b9d9bf2b8d 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -21,6 +21,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/xact.h" #include "executor/executor.h" diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 84ac2e63add..e18bc2a42e2 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -37,6 +37,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/xact.h" #include "catalog/catalog.h" diff --git a/src/backend/executor/nodeSamplescan.c b/src/backend/executor/nodeSamplescan.c index c75f9f3a75b..7d4f17b4e99 100644 --- a/src/backend/executor/nodeSamplescan.c +++ b/src/backend/executor/nodeSamplescan.c @@ -15,6 +15,7 @@ #include "postgres.h" #include "access/hash.h" +#include "access/heapam.h" #include "access/relscan.h" #include "access/tsmapi.h" #include "executor/executor.h" diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c index 8c46873daaf..e5482859efc 100644 --- a/src/backend/executor/nodeSeqscan.c +++ b/src/backend/executor/nodeSeqscan.c @@ -27,6 +27,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/relscan.h" #include "executor/execdebug.h" #include "executor/nodeSeqscan.h" diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index 9961e2b2c1a..b7a8725e2dc 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -22,6 +22,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/sysattr.h" #include "catalog/pg_type.h" #include "executor/execdebug.h" diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index b849ae03b83..8cdcf2368b2 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -18,6 +18,7 @@ #include <limits.h> #include <math.h> +#include "access/heapam.h" #include "access/htup_details.h" #include "access/parallel.h" #include "access/sysattr.h" diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index dfbc1cc4995..e6af7a431f7 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -16,6 +16,7 @@ #include <ctype.h> +#include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" #include "catalog/heap.h" diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index f3530c3a541..bebee9d540b 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -27,6 +27,7 @@ #include "postgres.h" #include "access/amapi.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/reloptions.h" #include "catalog/dependency.h" diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 71446963a44..60993c3a7a5 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -13,6 +13,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "catalog/partition.h" #include "catalog/pg_inherits.h" #include "catalog/pg_type.h" diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index adfa48e3ff8..d87cf8afe56 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -88,6 +88,7 @@ #include "miscadmin.h" #include "pgstat.h" +#include "access/heapam.h" #include "access/xact.h" #include "catalog/pg_subscription_rel.h" diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index f5d622193c2..de23ced9aff 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -27,6 +27,7 @@ #include "pgstat.h" #include "funcapi.h" +#include "access/heapam.h" #include "access/xact.h" #include "access/xlog_internal.h" diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index c7a5e630b77..4e5dcabf160 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -20,6 +20,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/sysattr.h" #include "catalog/dependency.h" #include "catalog/pg_type.h" diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 746b7d2fbac..f4d3eab2ea8 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -19,6 +19,7 @@ #include <math.h> #include <unistd.h> +#include "access/heapam.h" #include "access/sysattr.h" #include "catalog/catalog.h" #include "catalog/pg_tablespace.h" diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 590df56a0a4..e606eb342fd 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -30,6 +30,7 @@ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" #include "access/xact.h" diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 77811f6818a..8ed72bd2683 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -20,6 +20,7 @@ #include <fcntl.h> #include "access/amapi.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" #include "catalog/dependency.h" diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index e8f51d2d0d4..27e5bfef36c 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -103,6 +103,7 @@ #include "access/brin.h" #include "access/gin.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" #include "catalog/index.h" diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c index 1cec168b2a1..1db560cd66d 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -67,6 +67,7 @@ #endif #endif /* USE_LIBXML */ +#include "access/heapam.h" #include "access/htup_details.h" #include "catalog/namespace.h" #include "catalog/pg_class.h" diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 06503bc98b2..df30bff6c99 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -31,6 +31,7 @@ #include <unistd.h> #include "access/hash.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "access/multixact.h" #include "access/nbtree.h" diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c index 7199c2119a5..8fba7791cf8 100644 --- a/src/backend/utils/fmgr/funcapi.c +++ b/src/backend/utils/fmgr/funcapi.c @@ -13,6 +13,7 @@ */ #include "postgres.h" +#include "access/heapam.h" #include "access/htup_details.h" #include "catalog/namespace.h" #include "catalog/pg_proc.h" diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 17ab75fd3d6..cf2ac151241 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -33,21 +33,6 @@ typedef struct BulkInsertStateData *BulkInsertState; -/* - * Possible lock modes for a tuple. - */ -typedef enum LockTupleMode -{ - /* SELECT FOR KEY SHARE */ - LockTupleKeyShare, - /* SELECT FOR SHARE */ - LockTupleShare, - /* SELECT FOR NO KEY UPDATE, and UPDATEs that don't modify key columns */ - LockTupleNoKeyExclusive, - /* SELECT FOR UPDATE, UPDATEs that modify key columns, and DELETE */ - LockTupleExclusive -} LockTupleMode; - #define MaxLockTupleMode LockTupleExclusive /* diff --git a/src/include/access/hio.h b/src/include/access/hio.h index 84174766b98..cec087cb1a5 100644 --- a/src/include/access/hio.h +++ b/src/include/access/hio.h @@ -14,7 +14,6 @@ #ifndef HIO_H #define HIO_H -#include "access/heapam.h" #include "access/htup.h" #include "utils/relcache.h" #include "storage/buf.h" @@ -39,7 +38,7 @@ extern void RelationPutHeapTuple(Relation relation, Buffer buffer, HeapTuple tuple, bool token); extern Buffer RelationGetBufferForTuple(Relation relation, Size len, Buffer otherBuffer, int options, - BulkInsertState bistate, + BulkInsertStateData *bistate, Buffer *vmbuffer, Buffer *vmbuffer_other); #endif /* HIO_H */ diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h index 7bd32e18524..43a2286c172 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -15,7 +15,6 @@ #define RELSCAN_H #include "access/genam.h" -#include "access/heapam.h" #include "access/htup_details.h" #include "access/itup.h" #include "access/tupdesc.h" @@ -71,7 +70,7 @@ typedef struct HeapScanDescData BlockNumber rs_cblock; /* current block # in scan, if any */ Buffer rs_cbuf; /* current buffer in scan, if any */ /* NB: if rs_cbuf is not InvalidBuffer, we hold a pin on that buffer */ - ParallelHeapScanDesc rs_parallel; /* parallel scan information */ + struct ParallelHeapScanDescData *rs_parallel; /* parallel scan information */ /* these fields only used in page-at-a-time mode and for bitmap scans */ int rs_cindex; /* current tuple's index in vistuples */ @@ -155,7 +154,7 @@ typedef struct SysScanDescData { Relation heap_rel; /* catalog being scanned */ Relation irel; /* NULL if doing heap scan */ - HeapScanDesc scan; /* only valid in heap-scan case */ + struct HeapScanDescData *scan; /* only valid in heap-scan case */ IndexScanDesc iscan; /* only valid in index-scan case */ Snapshot snapshot; /* snapshot to unregister at end of scan */ } SysScanDescData; diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index 0f1f63b38e8..8daac5663cd 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -117,7 +117,7 @@ extern double IndexBuildHeapScan(Relation heapRelation, bool allow_sync, IndexBuildCallback callback, void *callback_state, - HeapScanDesc scan); + struct HeapScanDescData *scan); extern double IndexBuildHeapRangeScan(Relation heapRelation, Relation indexRelation, IndexInfo *indexInfo, @@ -127,7 +127,7 @@ extern double IndexBuildHeapRangeScan(Relation heapRelation, BlockNumber end_blockno, IndexBuildCallback callback, void *callback_state, - HeapScanDesc scan); + struct HeapScanDescData *scan); extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 50a943b6183..da0948e1a68 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -15,6 +15,7 @@ #define EXECUTOR_H #include "executor/execdesc.h" +#include "nodes/lockoptions.h" #include "nodes/parsenodes.h" #include "utils/memutils.h" @@ -180,15 +181,15 @@ extern void ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); -extern LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo); +extern enum LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo); extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti, bool missing_ok); extern ExecAuxRowMark *ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist); extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate, - Relation relation, Index rti, int lockmode, + Relation relation, Index rti, LockTupleMode lockmode, ItemPointer tid, TransactionId priorXmax); extern HeapTuple EvalPlanQualFetch(EState *estate, Relation relation, - int lockmode, LockWaitPolicy wait_policy, ItemPointer tid, - TransactionId priorXmax); + LockTupleMode lockmode, LockWaitPolicy wait_policy, + ItemPointer tid, TransactionId priorXmax); extern void EvalPlanQualInit(EPQState *epqstate, EState *estate, Plan *subplan, List *auxrowmarks, int epqParam); extern void EvalPlanQualSetPlan(EPQState *epqstate, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index a93bb61bf51..57031654900 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -15,7 +15,6 @@ #define EXECNODES_H #include "access/genam.h" -#include "access/heapam.h" #include "access/tupconvert.h" #include "executor/instrument.h" #include "lib/pairingheap.h" @@ -1268,7 +1267,7 @@ typedef struct ScanState { PlanState ps; /* its first field is NodeTag */ Relation ss_currentRelation; - HeapScanDesc ss_currentScanDesc; + struct HeapScanDescData *ss_currentScanDesc; TupleTableSlot *ss_ScanTupleSlot; } ScanState; diff --git a/src/include/nodes/lockoptions.h b/src/include/nodes/lockoptions.h index c293595ccb8..8e8ccff43ca 100644 --- a/src/include/nodes/lockoptions.h +++ b/src/include/nodes/lockoptions.h @@ -43,4 +43,19 @@ typedef enum LockWaitPolicy LockWaitError } LockWaitPolicy; +/* + * Possible lock modes for a tuple. + */ +typedef enum LockTupleMode +{ + /* SELECT FOR KEY SHARE */ + LockTupleKeyShare, + /* SELECT FOR SHARE */ + LockTupleShare, + /* SELECT FOR NO KEY UPDATE, and UPDATEs that don't modify key columns */ + LockTupleNoKeyExclusive, + /* SELECT FOR UPDATE, UPDATEs that modify key columns, and DELETE */ + LockTupleExclusive +} LockTupleMode; + #endif /* LOCKOPTIONS_H */ -- 2.18.0.rc2.dirty
