Re: Fix typos and inconsistencies for v16 - Mailing list pgsql-hackers

Hi David,

21.04.2023 01:49, David Rowley wrote:
> On Wed, 19 Apr 2023 at 07:00, Alexander Lakhin <exclusion@gmail.com> wrote:
>> please look at the similar list for v15+ (596b5af1d..HEAD).
> I've now pushed most of these but didn't include the following ones:

Thank you!

>> 3. BufFileOpenShared -> BufFileOpenFileSet // see dcac5e7ac
> Maybe I need to spend longer, but I just didn't believe the command
> that claimed that "BufFiles opened using BufFileOpenFileSet() are
> read-only by definition". Looking at the code for that, it seems to
> depend on if O_RDONLY is included in the mode flags.

I've found the following explanation for that:
1) As of dcac5e7ac~1 function ltsConcatWorkerTapes() contained:
...
                 file = BufFileOpenShared(fileset, filename, O_RDONLY);
...
          * The only thing that currently prevents writing to the leader tape from
          * working is the fact that BufFiles opened using BufFileOpenShared() are
          * read-only by definition, but that could be changed if it seemed
...

2) A patch [1], which eventually resulted in c4649cce3, initially started
with the change:

-ltsConcatWorkerTapes(LogicalTapeSet *lts, TapeShare *shared,
...
-     * working is the fact that BufFiles opened using BufFileOpenShared() are
...
+LogicalTapeImport(LogicalTapeSet *lts, int worker, TapeShare *shared)
...
+    file = BufFileOpenShared(lts->fileset, filename, O_RDONLY);
...
+     * the fact that BufFiles opened using BufFileOpenShared() are read-only

3) The commit dcac5e7ac (pushed 2021-08-30) renamed the function
BufFileOpenShared() to BufFileOpenFileSet() and changed the comment:
...
          * The only thing that currently prevents writing to the leader tape from
-        * working is the fact that BufFiles opened using BufFileOpenShared() are
+        * working is the fact that BufFiles opened using BufFileOpenFileSet() are
          * read-only by definition, but that could be changed if it seemed
...

4) The commit c4649cce3 (pushed 2021-10-18) removed the comment referencing
BufFileOpenFileSet() and added that somewhat distant comment
referencing BufFileOpenShared():
$ git show c4649cce3 src/backend/utils/sort/logtape.c | grep 'BufFiles opened'
-        * working is the fact that BufFiles opened using BufFileOpenFileSet() are
+        * the fact that BufFiles opened using BufFileOpenShared() are read-only

So I still believe that the "BufFileOpenShared -> BufFileOpenFileSet" change
is correct and that comment can be read now as referencing to the line:
     file = BufFileOpenFileSet(<s->fileset->fs, filename, O_RDONLY, false);
in LogicalTapeImport(). Although it could be improved, for sure.

Please look at the following two bunches for v14+ and v13+ (split to ease
back-patching if needed). Having processed them, I've reached the state that
could be considered "clean" ([2], [3]); at least I don't see how to detect
yet more errors of this class in dozens, so it's my last run for now (though I
have several entities left, which I couldn't find replacements for).

v14+:
1. AsyncCtl -> NotifyCtl // renamed in 5da14938f
2. ATExecConstrRecurse -> ATExecAlterConstrRecurse
3. attlocal -> attislocal
4. before_shmem_access -> before_shmem_exit
5. bodys -> bodies
6. can_getnextslot_tidrange -> scan_getnextslot_tidrange
7. DISABLE_ATOMICS -> HAVE_ATOMICS
8. FETCH_H -> REWIND_SOURCE_H
9. filed -> field
10. find_minmax_aggs_walker -> can_minmax_aggs // renamed in 0a2bc5d61e

11. GroupExprInfo ->  GroupVarInfo //// a4d75c86b
12. LD_DEAD -> LP_DEAD
13. libpq-trace.c -> fe-trace.c
14. lowerItem -> lowestItem //// bb437f995
15. has_privs -> has_privs_of_role
16. heap_hot_prune_opt -> heap_page_prune_opt
17. MAX_CONVERSION_LENGTH -> MAX_CONVERSION_INPUT_LENGTH //// ea1b99a66
18. MAX_FLUSH_BUFFERS -> MAX_WRITEALL_BUFFERS // renamed in dee663f78
19. myscheam -> myschema // doc/ -- maybe should be backpatched
20. pgbestat_beinit -> pgstat_beinit

21. pgWALUsage -> pgWalUsage
22. point-in-time-recovery -> point-in-time recovery
23. PQnotify -> PGnotify
24. QUERYJUBLE_H -> QUERYJUMBLE_H
25. rd_partdesc_nodetach -> rd_partdesc_nodetached
26. ReadNewTransactionid -> GetNewTransactionId
27. RelationBuildDescr -> RelationBuildDesc
28. SnapBuildCommittedTxn -> SnapBuildCommitTxn // see DecodeCommit()
29. subscription_rel -> pg_subscription_rel
30. tap_rep -> tab_rep

31. total_heap_blks -> heap_blks_total
32. tuple_cids -> tuplecids
33. WatchLatch -> WaitLatch
34. WriteAll -> SimpleLruWriteAll
35. PageIsPrunable -- remove // that define and the PageIsPrunable() check above were removed in dc7420c2c

Candidates for removal:
BARRIER_SHOULD_CHECK //unused since a3ed4d1ef
EXE_EXT // unused since f06b1c598
get_toast_for // unused since 860593ec3
SizeOfCommitTsSet // unused since 08aa89b32

v13+:
1. agg_init_trans_check -> agg_trans
2. agg_strict_trans_check -> agg_trans
3. amopclassopts -> amoptsprocnum //// since 911e70207
4. CommitTSBuffer -> CommitTsBuffer // the inconsistency exists since 5da14938f; maybe this change should be
backpatched
5. gist_intbig_ops -> gist__intbig_ops
6. gist_int_ops -> gist__int_ops
7. laftleft -> lastleft
8. lc_message -> locale_name // in accordance with the search_locale_enum() description
9. leftype -> lefttype
10. mksafefunc -> mkfunc // see 1f474d299

11. openSegment -> segment_open // in accordance with the WALRead() description
12. parse_util.c -> parse_utilcmd.c
13. process_innerer_partition -> process_inner_partition
14. read_spilled_tuple -> hashagg_batch_read
15. SortGroupNode -> SortGroupClause
16. SWITCH_WAL -> XLOG_SWITCH
17. tts_attr -> ttc_attr
18. tts_oldvalues -> ttc_oldvalues
19. tts_oldisnull -> ttc_oldisnull
20. tts_rel -> ttc_rel

21. taget -> target
22. WALSnd -> WalSnd
23. XLogRoutine -> XLogReaderRoutine

Candidates for removal:
endterm // see 60c90c16c -- Use xreflabel attributes instead of endterm attributes ...
package_tarname // not used since introduction in 1933ae629
my $clearpass = "FooBaR1"; // unreferenced since b846091fd

[1] https://www.postgresql.org/message-id/91284957-3cb2-944e-5f14-5c2ff86b49fa%40iki.fi
(0001-Refactor-LogicalTapeSet-LogicalTape-interface.patch)

[2] https://www.postgresql.org/message-id/flat/5da8e325-c665-da95-21e0-c8a99ea61fbf%40gmail.com
[3]
https://www.postgresql.org/message-id/flat/CALDaNm0ni%2BGAOe4%2BfbXiOxNrVudajMYmhJFtXGX-zBPoN8ixhw%40mail.gmail.com

Best regards,
Alexander