2)
+ /*
+ * Check if the old tuple's attribute is stored externally and is a
+ * member of external_cols.
+ */
+ if (VARATT_IS_EXTERNAL((struct varlena *) DatumGetPointer(value1)) &&
+ bms_is_member(attrnum - FirstLowInvalidHeapAttributeNumber,
+ external_cols))
+ *has_external = true;
If has_external is already true, it seems we don't need this check, so should we
check has_external first?
Is it worth it? I don't think so. It complicates a non-critical path. In
general, the condition will be executed once or twice.
