On Tue, Nov 11, 2025 at 7:14 AM Sugamoto Shinya <[email protected]> wrote: > > Thanks everyone for checking my proposal. > Other than how to make the hint string, every comment looks good to me. I'll > take those into my patch. I appreciate it. > > > Given the comments you all made, I looked into the source code to see how we > build `errhint` with values, and found the following. > > // contrib/pg_prewarm/pg_prewarm.c:102 > errhint("Valid prewarm types are \"prefetch\", \"read\", and \"buffer\".") > > // contrib/amcheck/verify_heapam.c:303 > errhint("Valid skip options are \"all-visible\", \"all-frozen\", and > \"none\".") > > // src/backend/replication/logical/launcher.c:458 > errhint("You might need to increase \"%s\".", > "max_logical_replication_workers") > > // src/backend/commands/opclasscmds.c:1240 > errhint("Valid signature of operator class options parsing function is %s.", > "(internal) RETURNS void"))); > > // src/backend/commands/dbcommands.c:1044 > errhint("Valid strategies are \"wal_log\" and \"file_copy\"."))); > > > Regarding the following comment, > > >In fact, the less often something changes, the easier it is to miss updating > >it when we need to. > > > > If we add a helper function to build the list of supported encodings: > > > > * The function is small and straightforward — trivial to implement. > > * The function doesn’t run in any performance-critical paths, so no > > meaningful cost there. > > * The encoding names are plain ASCII identifiers and effectively the same > > across languages, so there’s no real benefit to hardcoding them for i18n > > purposes either. > > While I partly agree with the idea of adding a helper function, introducing > one just for constructing a simple hint string would lead to more code of the > kind that I would prefer to avoid, since this is not our main concern. > > So, although it is true that having such a helper function would improve > maintainability in some sense, > considering that: > - there are already several existing cases where values are simply embedded > into hint texts (as Fujii pointed out), and > - this approach does not violate our coding rules or make the code harder to > understand, > I tend to think that it’s acceptable to hard-code the list of possible > encodings in the hint this time. > > Thoughts?
+1 for hard-coding the supported encoding list. I think it's a good start point. We can revisit the idea of dynamically constructing the encoding list if we're concerned about its maintenance cost. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
