On Mon, Nov 7, 2022 at 5:19 PM Peter Smith <smithpb2...@gmail.com> wrote:
> On Mon, Nov 7, 2022 at 5:50 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > Peter Smith <smithpb2...@gmail.com> writes: > > > Sorry, I forgot the attachments in the previous post. PSA. > > > > I spent a bit of time looking at this. I agree that a lot of the > > current ordering choices here look like they were made with the > > advice of a dartboard, and there's a number of things that are > > pretty blatantly just sloppy merging (like the out-of-order > > wait-event items). However, I'm not a big fan of "alphabetical > > order at all costs", because that frequently leads to ordering > > decisions that are not a lot better than random from a semantic > > standpoint. For example, I resist the idea that it's sensible > > to put pg_stat_all_indexes before pg_stat_all_tables. > > I'm unconvinced that putting pg_stat_sys_tables and > > pg_stat_user_tables far away from pg_stat_all_tables is great, > > either. > > > > Thanks for taking the time to look at my patch. The "at all costs" > approach was not the intention - I was just trying only to apply some > sane ordering where I did not recognize a reason for the current > order. > > > So ... how do we proceed? > > > > To proceed with the existing patches I need some guidance on exactly > which of the changes can be considered improvements versus which ones > are maybe just trading one 'random' order for another. > > How about below? > > Table 28.1. Dynamic Statistics Views -- Alphabetical order would be a > small improvement here, right? > The present ordering seems mostly OK, though just like the "Progress" update below the bottom 6 pg_stat_progress_* entries should be alphabetized; but leaving them as a group at the end seems desirable. Move pg_stat_recovery_prefetch either after subscription or after activity - the replication/received/subscription stuff all seems like it should be grouped together. As well as the security related ssl/gssapi. > Table 28.2. Collected Statistics Views -- Leave this one unchanged > (per your comments above). > I would suggest moving the 3 pg_statio_*_tables rows between the pg_stat_*_tables and the pg_stat_xact_*_tables groups. Everything pertaining to cluster, database, tables, indexes, functions. slru and replication slots should likewise shift to the (near) top in the cluster/database grouping. > Table 28.12 Wait Events of type LWLock -- Seems a clear case of bad > merging. Alphabetical order is surely needed here, right? > +1 Agreed. > > Table 28.34 Additional Statistic Functions -- Alphabetical order would > be a small improvement here, right? > No. All "reset" items should be grouped at the end like they are. I don't see an alternative ordering among them that is clearly superior. Same for the first four. > > Table 28.35 Per-Backend Statistics Functions -- Alphabetical order > would be a small improvement here, right? > > This one I would rearrange alphabetically. Or, at least, I have a different opinion of what would make a decent order but it doesn't seem all that clearly better than alphabetical. > > I'd be inclined to alphabetize by SQL command name, but maybe > > leave Base Backup to the end since it's not a SQL command. > > > > Yes, I had previously only looked at the content of section 28.2 > because I didn't want to get carried away by changing too much until > there was some support for doing the first part. > > Now PSA a separate patch for fixing section "28.4. Progress Reporting" > order as suggested. > > This seems like a clear win. David J.