Em ter., 17 de jun. de 2025 às 10:43, Tomas Vondra <tomas@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.
