On Fri, Feb 17, 2017 at 2:28 AM, David Christensen <da...@endpoint.com> wrote: > - Change "data_checksums" from a simple boolean to "data_checksum_state", an > enum type for all of > the potentially-required states for this feature (as well as enabling).
Color me skeptical. I don't know what CHECKSUMS_ENABLING, CHECKSUMS_ENFORCING, and CHECKSUMS_REVALIDATING are intended to represent -- and there's no comments in the patch explaining it -- but if we haven't yet written the code to enable checksums, how do we know for sure which states it will require? If we're going to accept a patch to disable checksums without also having the capability to enable checksums, I think we should leave out the speculative elements about what might be needed on the "enable" side and just implement the minimal "disable" side. However, FWIW, I don't accept that being able to disable checksums online is a sufficient advance to justify enabling checksums by default. Tomas had some good points on another thread about what might be needed to really make that a good choice, and I'm still skeptical about whether checksums catch any meaningful number of errors that wouldn't be caught otherwise, and about the degree to which any complaints it issues are actionable. I'm not really against this patch on its own merits, but I think it's a small advance in an area that needs a lot of work. I think it would be a lot more useful if we had a way to *enable* checksums online. Then people who find out that checksums exist and want them have an easier way of getting them, and anyone who uses the functionality in this patch and then regrets it has a way to get back. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
