Hi, On 2019-11-13 15:58:28 +0900, Michael Paquier wrote: > On Tue, Nov 12, 2019 at 10:33:16AM -0800, Andres Freund wrote: > > On 2019-11-12 16:25:06 +0100, Daniel Gustafsson wrote: > >> On 11 Nov 2019, at 09:32, Michael Paquier <mich...@paquier.xyz> wrote: > >> > >>> This part has resulted in 75c1921, and we could just change > >>> DecodeMultiInsert() so as if there is no tuple data then we'd just > >>> leave. However, I don't feel completely comfortable with that either > >>> as it would be nice to still check that for normal relations we > >>> *always* have a FPW available. > >> > >> XLH_INSERT_CONTAINS_NEW_TUPLE will only be set in case of catalog relations > >> IIUC (that is, non logically decoded relations), so it seems to me that we > >> can > >> have a fastpath out of DecodeMultiInsert() which inspects that flag without > >> problems. Is this proposal along the lines of what you were thinking? > > > > Maybe I'm missing something, but it's the opposite, no? > > > bool need_tuple_data = RelationIsLogicallyLogged(relation); > > Yep. This checks after IsCatalogRelation(). > > > ... > > if (need_tuple_data) > > xlrec->flags |= XLH_INSERT_CONTAINS_NEW_TUPLE; > > > > or am I misunderstanding what you mean? > > Not sure that I can think about a good way to properly track if the > new tuple data is associated to a catalog or not, aka if the data is > expected all the time or not to get a good sanity check when doing the > multi-insert decoding. We could extend xl_multi_insert_tuple with a > flag to track that, but that seems like an overkill for a simple > sanity check..
My point was that I think there must be negation missing in Daniel's statement - XLH_INSERT_CONTAINS_NEW_TUPLE will only be set if *not* a catalog relation. But Daniel's statement says exactly the opposite, at least by my read. I can't follow what you're trying to get at in this sub discussion - why would we want to sanity check something about catalog tables here? Given that XLH_INSERT_CONTAINS_NEW_TUPLE is set exactly when the table is not a catalog table, that seems entirely superfluous? Greetings, Andres Freund
