From: | Alexander Lakhin <exclusion(at)gmail(dot)com> |
---|---|
To: | David Rowley <dgrowleyml(at)gmail(dot)com> |
Cc: | Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Fix typos and inconsistencies for v16 |
Date: | 2023-04-21 09:00:00 |
Message-ID: | e8c38840-596a-83d6-bd8d-cebc51111572@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi David,
21.04.2023 01:49, David Rowley wrote:
> On Wed, 19 Apr 2023 at 07:00, Alexander Lakhin <exclusion(at)gmail(dot)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
Attachment | Content-Type | Size |
---|---|---|
typo-fixes-for-v14+.1-10.patch | text/x-patch | 5.3 KB |
typo-fixes-for-v14+.11-20.patch | text/x-patch | 6.6 KB |
typo-fixes-for-v14+.21-30.patch | text/x-patch | 6.4 KB |
typo-fixes-for-v14+.31-35.patch | text/x-patch | 2.9 KB |
typo-fixes-for-v13+.1-10.patch | text/x-patch | 4.9 KB |
typo-fixes-for-v13+.11-20.patch | text/x-patch | 4.7 KB |
typo-fixes-for-v13+.21-23.patch | text/x-patch | 1.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2023-04-21 09:16:51 | Re: Support logical replication of DDLs |
Previous Message | Amul Sul | 2023-04-21 08:58:06 | Re: New committers: Nathan Bossart, Amit Langote, Masahiko Sawada |