Re: pgsql: Add pg_alterckey utility to change the cluster key

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Add pg_alterckey utility to change the cluster key
Date: 2020-12-26 17:18:18
Message-ID: 20201226171818.GO19054@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Sat, Dec 26, 2020 at 11:45:41AM -0500, Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > On Sat, Dec 26, 2020 at 06:16:37PM +0900, Michael Paquier wrote:
> >> The CF bot at http://cfbot.cputube.org/ includes tests on Windows, so
> >> those problems would have been detected beforehand. Did you look at
> >> these? If this cannot be fixed, could it be possible to revert
> >> please? It looks rather clear that this has not been tested across
> >> multiple platforms, and the absence of tests to allow the buildfarm to
> >> stress this code does not really help either in gaining confidence
> >> that this is stable.
>
> > I have been working on this patch for almost two months, and wanted to
> > try to get it into the tree near Christmas as sort of a Christmas
> > present to the community.
>
> It's not much of a "Christmas present" to have to spend part of the
> holiday fixing somebody else's obviously under-baked commits.

I thought I could just fix it on my own on that day.

> I think you should revert *all* of this and not come back until
> (a) you have some test cases and (b) somebody besides you has read
> the entire patch. There are way too many beginner-level errors in
> what you've pushed so far.

Magnus pointed out that one big help would have been to add it to the
commitfest, so the cfbot could have warned me of these issues. I
underestimated how much help that would have been. I am also using my
old workflow since I haven't worked on any large patches like this
lately. I guess I am relearning.

I can easily revert and come back, though the buildfarm is green now.
As far as testing, I can test that the cluster key unlocks the data
keys, but there is no current interface to the data keys. Ideally we
would test the full input/output path, but with no access to the output,
how can we test it? Also, as I stated, there are some shell script APIs
that can't easily be tested, e.g. AWS, Yubikey. I can continue to test
those manually.

> And that's not even counting the question
> of whether the design is right to begin with, which somebody already
> expressed doubts about.

What doubts?

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Bruce Momjian 2020-12-26 19:00:02 Re: pgsql: Add pg_alterckey utility to change the cluster key
Previous Message Tom Lane 2020-12-26 16:45:41 Re: pgsql: Add pg_alterckey utility to change the cluster key

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2020-12-26 18:00:16 Re: Rethinking plpgsql's assignment implementation
Previous Message Zhihong Yu 2020-12-26 16:53:28 Re: pg_stat_statements and "IN" conditions