Em ter., 17 de jun. de 2025 às 10:43, Tomas Vondra <to...@vondra.me> escreveu:
> On 6/17/25 15:09, Aleksander Alekseev wrote: > > Hi, > > > >>> We would be in serious trouble if RelationReloadNailed() could be > >>> called for a non-existing relation. Wouldn't Assert() be more > >>> appropriate? > >> > >> IMO, I think no. > >> > >> In all paths, this check is done, why would this be the only exception? > > > > We use Asserts() for cases that shouldn't happen in practice for > > performance reasons. Since this code doesn't crash I suspect this is > > one of such cases. Unless you are aware of a specific scenario that > > makes the code crash of course. > > > > Right. Assert() are for checks that we don't expect to ever fail, so we > don't want to keep them in production builds. I believe this is exactly > such case, because the code is about "nailed" catalogs, that we need to > access very early (e.g. before being able to access pg_class). > > There's only ~10 of such catalogs (look for formrdesc calls), and if we > get a failure here, it's not clear to me we can really keep the system > running anyway. It's not just the usual "relcache miss" and if the user > retries it will probably work fine. The catalog is borked, and who knows > in what way. > > My opinion is that adding a "elog(ERROR)" here would be misleading, as > it implies it's something we expect. And mostly pointless. I can imagine > adding an Assert, but I don't quite see how is that better than just > hitting a segfault a couple lines later. > To me this is a contradiction, whether you consider waiting for a segfault or consider adding an Assert. For the user it is better to have a log, where he can quickly find the problem, rather than having to investigate on his own. best regards, Ranier Vilela
