В письме от понедельник, 24 марта 2025 г. 23:04:39 MSK пользователь Nathan Bossart написал: > > We can have isset_offset, but then we have redesign all options with > > custom unset behavior to use it, instead of unreachable default value. > > This will make it consistent then. > > I don't see any reason why we are compelled to redesign all such options, > but in any case, I would think that would be preferable to magic special > values. For example, > > /* -1 is used to disable max threshold */ > vac_max_thresh = (relopts && relopts->vacuum_max_threshold >= -1) > ? relopts->vacuum_max_threshold > > : autovacuum_vac_max_thresh; > > would become something like > > if (relopts && relopts->vacuum_max_threshold_set) > vac_max_thresh = relopts->vacuum_max_threshold; > else > vac_max_thresh = autovacuum_vac_max_thresh;
I totally agree. This looks much nicer. But see how the code would change for fillfactor, where default value is in valid range and to be actually used? What would happen to default_val attribute in relopt_* structures? Should we keep it? Should we remove it? If we keep it, what should we set for vacuum_max_threshold? If we remove it how we should treat default value for fillfactor? Then you see, you will have extra int field for at least for every option with unreachable defaults. Do we really need this? default_val and isset_* flags are two completely different ways to do similar thing. You will never find a nice way to use them both. So I am up to keeping it the way it is: there exist only default_val and it is used to detect unset option when needed. > But again, I don't see any strong reason why we must change all such > reloptions. Because code of the engine should be consistent. We can't have two different ways to do same thing. If we have isset flag, we should go for it everywhere, where isset logic exists. Or do not use it at all. Other ways will lead us to a much bigger mess, then we have today. -- Nikolay Shaplov aka Nataraj Fuzzing Engineer at Postgres Professional Matrix IM: @dhyan:nataraj.su
signature.asc
Description: This is a digitally signed message part.
