On Wed, 22 Apr 2020 at 23:50, Michael Paquier <mich...@paquier.xyz> wrote: > > On Wed, Apr 22, 2020 at 10:21:21PM +1200, David Rowley wrote: > > I pushed a patch to remove the Assert. I didn't really feel a need to > > make any adjustments to the regression tests for this. The Assert was > > clearly out of place, it's hard to imagine that this could ever get > > broken again. > > Still, it seems to me that there could be a point in having a test for > partitioned tables with FKs referencing themselves. We have such > tests for plain tables for example.
The reason I didn't take the additional tests that were proposed by Amul was that I didn't think that they really added any additional coverage to the code that remained. They would only have served to ensure that nobody went and added the same Assert back again. Since the Assert was clearly out of place, I didn't think it was worthy of burdening our build farm with the additional overhead of the modified test from now until the end of time. I'd put it akin to fixing a spelling mistake in an error message and adding a special test specifically to ensure the spelling is correct. Would we add a test for that? Likely not. Would someone reintroduce the spelling mistake again? Likely not. I think discussions for any tests beyond the scope of this fix are probably for another thread. I'm happy to join any discussions about it there. David
