Обсуждение: pgsql: Rename files and headers related to index AM
Rename files and headers related to index AM The following renaming is done so as source files related to index access methods are more consistent with table access methods (the original names used for index AMs ware too generic, and could be confused as including features related to table AMs): - amapi.h -> indexam.h. - amapi.c -> indexamapi.c. Here we have an equivalent with backend/access/table/tableamapi.c. - amvalidate.c -> indexamvalidate.c. - amvalidate.h -> indexamvalidate.h. - genam.c -> indexgenam.c. - genam.h -> indexgenam.h. This has been discussed during the development of v12 when table AM was worked on, but the renaming never happened. Author: Michael Paquier Reviewed-by: Fabien Coelho, Julien Rouhaud Discussion: https://postgr.es/m/20191223053434.GF34339@paquier.xyz Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/8ce3aa9b5914d1ac45ed3f9bc484f66b3c4850c7 Modified Files -------------- contrib/bloom/blinsert.c | 2 +- contrib/bloom/bloom.h | 2 +- contrib/bloom/blutils.c | 2 +- contrib/bloom/blvacuum.c | 2 +- contrib/bloom/blvalidate.c | 2 +- contrib/sepgsql/database.c | 2 +- contrib/sepgsql/label.c | 2 +- contrib/sepgsql/proc.c | 2 +- contrib/sepgsql/relation.c | 2 +- contrib/sepgsql/schema.c | 2 +- src/backend/access/brin/brin_inclusion.c | 2 +- src/backend/access/brin/brin_minmax.c | 2 +- src/backend/access/brin/brin_validate.c | 2 +- src/backend/access/common/detoast.c | 2 +- src/backend/access/common/toast_internals.c | 2 +- src/backend/access/gin/ginvalidate.c | 2 +- src/backend/access/gist/gistbuild.c | 2 +- src/backend/access/gist/gistbuildbuffers.c | 2 +- src/backend/access/gist/gistget.c | 2 +- src/backend/access/gist/gistvacuum.c | 2 +- src/backend/access/gist/gistvalidate.c | 2 +- src/backend/access/hash/hashvalidate.c | 2 +- src/backend/access/heap/heapam.c | 2 +- src/backend/access/heap/heapam_handler.c | 2 +- src/backend/access/heap/vacuumlazy.c | 2 +- src/backend/access/index/Makefile | 8 ++++---- src/backend/access/index/indexam.c | 2 +- src/backend/access/index/{amapi.c => indexamapi.c} | 6 +++--- .../access/index/{amvalidate.c => indexamvalidate.c} | 6 +++--- src/backend/access/index/{genam.c => indexgenam.c} | 8 ++++---- src/backend/access/nbtree/nbtvalidate.c | 2 +- src/backend/access/spgist/spgdoinsert.c | 2 +- src/backend/access/spgist/spginsert.c | 2 +- src/backend/access/spgist/spgscan.c | 2 +- src/backend/access/spgist/spgutils.c | 2 +- src/backend/access/spgist/spgvacuum.c | 2 +- src/backend/access/spgist/spgvalidate.c | 2 +- src/backend/bootstrap/bootstrap.c | 2 +- src/backend/catalog/aclchk.c | 2 +- src/backend/catalog/catalog.c | 2 +- src/backend/catalog/dependency.c | 2 +- src/backend/catalog/heap.c | 2 +- src/backend/catalog/index.c | 2 +- src/backend/catalog/indexing.c | 2 +- src/backend/catalog/objectaddress.c | 2 +- src/backend/catalog/partition.c | 2 +- src/backend/catalog/pg_collation.c | 2 +- src/backend/catalog/pg_constraint.c | 2 +- src/backend/catalog/pg_db_role_setting.c | 2 +- src/backend/catalog/pg_depend.c | 2 +- src/backend/catalog/pg_enum.c | 2 +- src/backend/catalog/pg_inherits.c | 2 +- src/backend/catalog/pg_largeobject.c | 2 +- src/backend/catalog/pg_publication.c | 2 +- src/backend/catalog/pg_range.c | 2 +- src/backend/catalog/pg_shdepend.c | 2 +- src/backend/catalog/pg_subscription.c | 2 +- src/backend/commands/analyze.c | 2 +- src/backend/commands/cluster.c | 2 +- src/backend/commands/comment.c | 2 +- src/backend/commands/constraint.c | 2 +- src/backend/commands/dbcommands.c | 2 +- src/backend/commands/extension.c | 2 +- src/backend/commands/functioncmds.c | 2 +- src/backend/commands/indexcmds.c | 2 +- src/backend/commands/matview.c | 2 +- src/backend/commands/opclasscmds.c | 2 +- src/backend/commands/policy.c | 2 +- src/backend/commands/proclang.c | 2 +- src/backend/commands/publicationcmds.c | 2 +- src/backend/commands/seclabel.c | 2 +- src/backend/commands/tablecmds.c | 2 +- src/backend/commands/trigger.c | 2 +- src/backend/commands/tsearchcmds.c | 2 +- src/backend/commands/typecmds.c | 2 +- src/backend/commands/user.c | 2 +- src/backend/commands/vacuum.c | 2 +- src/backend/executor/execAmi.c | 2 +- src/backend/executor/execCurrent.c | 2 +- src/backend/executor/execIndexing.c | 2 +- src/backend/executor/execReplication.c | 2 +- src/backend/executor/nodeBitmapIndexscan.c | 2 +- src/backend/executor/nodeIndexonlyscan.c | 2 +- src/backend/optimizer/path/costsize.c | 4 ++-- src/backend/optimizer/plan/planner.c | 2 +- src/backend/optimizer/util/plancat.c | 2 +- src/backend/parser/parse_utilcmd.c | 2 +- src/backend/partitioning/partdesc.c | 2 +- src/backend/replication/logical/origin.c | 2 +- src/backend/rewrite/rewriteRemove.c | 2 +- src/backend/statistics/extended_stats.c | 2 +- src/backend/storage/large_object/inv_api.c | 2 +- src/backend/utils/adt/amutils.c | 2 +- src/backend/utils/adt/enum.c | 2 +- src/backend/utils/adt/ruleutils.c | 2 +- src/backend/utils/cache/catcache.c | 2 +- src/backend/utils/cache/evtcache.c | 2 +- src/backend/utils/cache/relfilenodemap.c | 2 +- src/backend/utils/cache/ts_cache.c | 2 +- src/backend/utils/init/postinit.c | 2 +- src/include/access/brin_internal.h | 2 +- src/include/access/gin_private.h | 2 +- src/include/access/gist_private.h | 4 ++-- src/include/access/gistscan.h | 2 +- src/include/access/hash.h | 2 +- src/include/access/{amapi.h => indexam.h} | 14 +++++++------- src/include/access/{amvalidate.h => indexamvalidate.h} | 12 ++++++------ src/include/access/{genam.h => indexgenam.h} | 14 +++++++------- src/include/access/nbtree.h | 2 +- src/include/access/reloptions.h | 2 +- src/include/access/spgist.h | 2 +- src/include/executor/nodeIndexscan.h | 2 +- src/include/nodes/nodes.h | 2 +- src/include/nodes/pathnodes.h | 2 +- src/include/utils/index_selfuncs.h | 4 ++-- src/include/utils/rel.h | 2 +- src/test/modules/dummy_index_am/dummy_index_am.c | 2 +- 117 files changed, 147 insertions(+), 147 deletions(-)
On 2019-Dec-25, Michael Paquier wrote: > Rename files and headers related to index AM > > The following renaming is done so as source files related to index > access methods are more consistent with table access methods (the > original names used for index AMs ware too generic, and could be > confused as including features related to table AMs): > - amapi.h -> indexam.h. > - amapi.c -> indexamapi.c. Here we have an equivalent with > backend/access/table/tableamapi.c. > - amvalidate.c -> indexamvalidate.c. > - amvalidate.h -> indexamvalidate.h. > - genam.c -> indexgenam.c. > - genam.h -> indexgenam.h. I think this commit was too hasty. There's too many people distracted by holidays. I suppose that this renaming is going to impact all third-party extensions that use those headers. If there are no complaints to this, that means we can do some more invasive header refactoring, unimpeded by such complaints either. https://postgr.es/m/20130918222610.GB342709@tornado.leadboat.com -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi, On 2019-12-26 11:46:06 -0300, Alvaro Herrera wrote: > On 2019-Dec-25, Michael Paquier wrote: > > > Rename files and headers related to index AM > > > > The following renaming is done so as source files related to index > > access methods are more consistent with table access methods (the > > original names used for index AMs ware too generic, and could be > > confused as including features related to table AMs): > > - amapi.h -> indexam.h. > > - amapi.c -> indexamapi.c. Here we have an equivalent with > > backend/access/table/tableamapi.c. > > - amvalidate.c -> indexamvalidate.c. > > - amvalidate.h -> indexamvalidate.h. > > - genam.c -> indexgenam.c. > > - genam.h -> indexgenam.h. > > I think this commit was too hasty. There's too many people distracted > by holidays. +1 > I suppose that this renaming is going to impact all third-party > extensions that use those headers. If there are no complaints to this, > that means we can do some more invasive header refactoring, unimpeded by > such complaints either. Yea, this seems like a much bigger move than warranted. Especially without a backward compat header put into place. Imo this ought to be reverted. Greetings, Andres Freund
On Thu, Dec 26, 2019 at 10:15 AM Andres Freund <andres@anarazel.de> wrote: > Yea, this seems like a much bigger move than warranted. Especially > without a backward compat header put into place. > > Imo this ought to be reverted. I agree. -- Peter Geoghegan
On Thu, Dec 26, 2019 at 10:26:51AM -0800, Peter Geoghegan wrote: > On Thu, Dec 26, 2019 at 10:15 AM Andres Freund <andres@anarazel.de> wrote: >> Yea, this seems like a much bigger move than warranted. Especially >> without a backward compat header put into place. Hm, I am not sure that it is actually that much used, such stuff is very specialized. And this does not prevent the renaming the of the source files. One consumer I can think about is Postgres Pro, like here: https://github.com/postgrespro/rum >> Imo this ought to be reverted. > > I agree. Anyway, reverted, let's keep this discussion for later. And sorry for the inconvenience. -- Michael
Вложения
Hi, On 2019-12-27 08:20:17 +0900, Michael Paquier wrote: > On Thu, Dec 26, 2019 at 10:26:51AM -0800, Peter Geoghegan wrote: > > On Thu, Dec 26, 2019 at 10:15 AM Andres Freund <andres@anarazel.de> wrote: > >> Yea, this seems like a much bigger move than warranted. Especially > >> without a backward compat header put into place. > > Hm, I am not sure that it is actually that much used, such stuff is > very specialized. That's true for some of this, but e.g. genam.h is pretty widely included. I mean, you had to adapt like 100+ files and while like 30 or so of those are in implementation details of individual indexes, the rest is not. Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2019-12-27 08:20:17 +0900, Michael Paquier wrote:
>> Hm, I am not sure that it is actually that much used, such stuff is
>> very specialized.
> That's true for some of this, but e.g. genam.h is pretty widely
> included. I mean, you had to adapt like 100+ files and while like 30 or
> so of those are in implementation details of individual indexes, the
> rest is not.
This may suggest that we should think about an actual refactoring,
rather than just mechanical renaming. Do these results mean that
we've allowed index API details to bleed into the wrong places?
(Probably ought to take this thread to -hackers if anyone wants
to discuss that.)
regards, tom lane