On 2022-Mar-07, Greg Stark wrote: > I don't think this is worth spending time adding tests for. I get what > you're saying that this is at least semi-intentional behaviour and > desirable behaviour so it should have tests ensuring that it continues > to work. But it's not documented behaviour and the test is basically > testing that the implementation is this specific implementation. > > I don't think the test is really a bad idea but neither is it really > very useful and I think it's not worth having people spend time > reviewing and discussing. I'm leaning to saying this patch is > rejected.
I've pushed this patch; I agree with Greg that this is semi-intentional and desirable, even if not documented. (I doubt we would document it anyway, as having previously SET an invalid option is not something that happens commonly; what reason would we have for documenting that it works to RESET such an option?) As for the implementation being this specific one, that's something already embedded in all of reloptions.sql, so I don't really mind that, if we do change it, we'll have to rewrite pretty much the whole file and not "the whole file except these three lines". Lastly, there is a long-running patch proposal to refactor reloptions quite significantly, so it'll be good to ensure that the new implementation doesn't break the features we already have; and this one corner is one thing Nikolay already said his patch had broken and failed to notice right away. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "La verdad no siempre es bonita, pero el hambre de ella sí"
