On 2018/09/04 21:51, Alvaro Herrera wrote: > On 2018-Sep-04, Amit Langote wrote: > >> On 2018/09/04 13:08, Alvaro Herrera wrote: > >>> I think it'd be pointless noise. If we really want to protect against >>> that, I think we should promote the Assert for relpartbound's isnull >>> flag into an if test. >> >> So that we can elog(ERROR, ...) if isnull is true? If so, I agree, >> because that's what should've been done here anyway (for a value read from >> the disk that is). > > Yes. > > I wonder now what's the value of using an Assert for this, BTW: if those > are enabled, then we'll crash saying "this column is null and shouldn't!". > If they are disabled, we'll instead crash (possibly in production) > trying to decode the field. What was achieved?
I can see how that the Assert was pointless all along. > With an elog(ERROR) the reaction is the expected one: the current > transaction is aborted, but the server continues to run. > >> I think we should check relispartition then too. > > Well, we don't check every single catalog flag in every possible code > path just to ensure it's sane. I don't see any reason to do differently > here. We rely heavily on the catalog contents to be correct, and > install defenses against user-induced corruption that would cause us to > crash; but going much further than that would be excessive IMO. Okay, sure anyway. Thanks, Amit
