Thread: Assorted small doc patches
Hackers,
I posted all of these elsewhere (docs, bugs) but am consolidating them here going forward.
v0001-database-default-name (-bugs, with a related cleanup suggestion as well)
v0002-doc-extension-dependent-routine-behavior (-general, reply to user confusion)
v0001-doc-savepoint-name-reuse (-docs, reply to user request for improvement)
v0001-on-conflict-excluded-is-name-not-table (-docs, figured out while trying to improve the docs to reduce user confusion in this area)
v0001-doc-make-row-estimation-example-match-prose (-docs, reply to user pointing of an inconsistency)
Thanks!
David J.
Attachment
On 2022-Apr-20, David G. Johnston wrote: > v0001-doc-savepoint-name-reuse (-docs, reply to user request for > improvement) > https://www.postgresql.org/message-id/CAKFQuwYzSb9OW5qTFgc0v9RWMN8bX83wpe8okQ7x6vtcmfA2KQ%40mail.gmail.com This one is incorrect; rolling back to a savepoint does not remove the savepoint, so if you ROLLBACK TO it again afterwards, you'll get the same one again. In fact, Your proposed example doesn't work as your comments intend. The way to get the effect you show is to first RELEASE the second savepoint, then roll back to the earliest one. Maybe like this: BEGIN; INSERT INTO table1 VALUES (1); SAVEPOINT my_savepoint; INSERT INTO table1 VALUES (2); SAVEPOINT my_savepoint; INSERT INTO table1 VALUES (3); ROLLBACK TO SAVEPOINT my_savepoint; SELECT * FROM table1; -- shows rows 1, 2 RELEASE SAVEPOINT my_savepoint; -- gets rid of the latest one without rolling back anything ROLLBACK TO SAVEPOINT my_savepoint; -- rolls back to the earliest one SELECT * FROM table1; -- just 1 COMMIT; -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
On Thu, Apr 21, 2022 at 10:46 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2022-Apr-20, David G. Johnston wrote:
> v0001-doc-savepoint-name-reuse (-docs, reply to user request for
> improvement)
> https://www.postgresql.org/message-id/CAKFQuwYzSb9OW5qTFgc0v9RWMN8bX83wpe8okQ7x6vtcmfA2KQ%40mail.gmail.com
This one is incorrect; rolling back to a savepoint does not remove the
savepoint, so if you ROLLBACK TO it again afterwards, you'll get the
same one again. In fact, Your proposed example doesn't work as your
comments intend.
Yeah, my bad for not testing things.
The way to get the effect you show is to first RELEASE the second
savepoint, then roll back to the earliest one. Maybe like this:
BEGIN;
INSERT INTO table1 VALUES (1);
SAVEPOINT my_savepoint;
INSERT INTO table1 VALUES (2);
SAVEPOINT my_savepoint;
INSERT INTO table1 VALUES (3);
ROLLBACK TO SAVEPOINT my_savepoint;
SELECT * FROM table1; -- shows rows 1, 2
RELEASE SAVEPOINT my_savepoint; -- gets rid of the latest one without rolling back anything
ROLLBACK TO SAVEPOINT my_savepoint; -- rolls back to the earliest one
SELECT * FROM table1; -- just 1
COMMIT;
I'm ok with that, though I decided to experiment a bit. I decided to use comments to make the example understandable without needing a server; self-contained AND easier to follow the status of both the table and the savepoint reference.
I explicitly demonstrate both release and rollback here along with the choice to use just a single savepoint name. We could make even more examples in a "unit test" type style but with the commentary I think this communicates the pertinent points quite well.
BEGIN;
INSERT INTO table1 VALUES (1);SAVEPOINT my_savepoint;
-- Savepoint: [1]; Table: [1]
INSERT INTO table1 VALUES (2);
SAVEPOINT my_savepoint;
-- Savepoint: [1,2]; Table: [1,2]
INSERT INTO table1 VALUES (3);
SAVEPOINT my_savepoint;
-- Savepoint: [1,2,3]; Table: [1,2,3]
INSERT INTO table1 VALUES (4);
-- Savepoint: [1,2,3]; Table: [1,2,3,4]
ROLLBACK TO SAVEPOINT my_savepoint;
-- Savepoint: [1,2,3]; Table: [1,2,3]
ROLLBACK TO SAVEPOINT my_savepoint; -- No Change
-- Savepoint: [1,2,3]; Table: [1,2,3]
SELECT * FROM table1;
RELEASE my_savepoint;
RELEASE my_savepoint;
-- Savepoint: [1]; Table: [1,2,3]
SELECT * FROM table1;
ROLLBACK TO SAVEPOINT my_savepoint;
-- Savepoint: [1]; Table: [1]
SELECT * FROM table1;
COMMIT;
David J.
Updated status of the set.
On Wed, Apr 20, 2022 at 5:59 PM David G. Johnston <david.g.johnston@gmail.com> wrote:
v0001-database-default-name (-bugs, with a related cleanup suggestion as well)
v0002-doc-extension-dependent-routine-behavior (-general, reply to user confusion)
v0001-doc-savepoint-name-reuse (-docs, reply to user request for improvement)
Pending discussion of alternate presentation of transaction sequence; if not favorable, I can just go with a simple factual fix of the mistake in v0001 (see this thread).
v0001-on-conflict-excluded-is-name-not-table (-docs, figured out while trying to improve the docs to reduce user confusion in this area)v0001-doc-make-row-estimation-example-match-prose (-docs, reply to user pointing of an inconsistency)
David J.
Anything I should be doing differently here to get a bit of reviewer/committer time on these? I'll add them to the commitfest for next month if needed but I'm seeing quick patches going in every week and the batch format done at the beginning of the month got processed through without issue.
I was hoping for Workflow A especially as I acquit myself more than adequately on the "How do you get someone to respond to you?" items.
I was going to chalk it up to bad timing but the volume of doc patches this month hasn't really dipped even with the couple of bad bugs being worked on.
Thank you!
David J.
On Fri, Apr 29, 2022 at 6:52 AM David G. Johnston <david.g.johnston@gmail.com> wrote:
Updated status of the set.On Wed, Apr 20, 2022 at 5:59 PM David G. Johnston <david.g.johnston@gmail.com> wrote:v0001-database-default-name (-bugs, with a related cleanup suggestion as well)
v0002-doc-extension-dependent-routine-behavior (-general, reply to user confusion)v0001-doc-savepoint-name-reuse (-docs, reply to user request for improvement)Pending discussion of alternate presentation of transaction sequence; if not favorable, I can just go with a simple factual fix of the mistake in v0001 (see this thread).v0001-on-conflict-excluded-is-name-not-table (-docs, figured out while trying to improve the docs to reduce user confusion in this area)v0001-doc-make-row-estimation-example-match-prose (-docs, reply to user pointing of an inconsistency)David J.
On 31.05.22 22:12, David G. Johnston wrote: > Anything I should be doing differently here to get a bit of > reviewer/committer time on these? I'll add them to the commitfest for > next month if needed but I'm seeing quick patches going in every week > and the batch format done at the beginning of the month got processed > through without issue. These patches appear to have merit, but they address various nontrivial areas of functionality, so either a) I just pick out a few that I understand and deal with those and leave the rest open, or b) I'm overwhelmed and do none. It might have been better to post these separately. I'll start with one though: v0001-database-default-name.patch I don't understand why you propose this change. It appears to reduce precision.
On Wed, Jun 1, 2022 at 7:05 AM Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
On 31.05.22 22:12, David G. Johnston wrote:
> Anything I should be doing differently here to get a bit of
> reviewer/committer time on these? I'll add them to the commitfest for
> next month if needed but I'm seeing quick patches going in every week
> and the batch format done at the beginning of the month got processed
> through without issue.
These patches appear to have merit, but they address various nontrivial
areas of functionality, so either a) I just pick out a few that I
understand and deal with those and leave the rest open, or b) I'm
overwhelmed and do none. It might have been better to post these
separately.
I did for quite a few of them, per the links provided. But I get your point, the originals weren't on -hackers for many of them and moving them over singly probably would have worked out better.
I'll start with one though: v0001-database-default-name.patch
I don't understand why you propose this change. It appears to reduce
precision.
As the proposed commit message says we don't tend to say "database user name" elsewhere in the documentation (or the error messages shown) so the removal of the word database doesn't actually change anything. We only need to provide a qualifier for user name when it is not the database user name that is being referred to, or basically when it is the operating system user name.
The last hunk is the actual bug - the existing wording effectively reads:
"The default database name is the operating system user name."
That is incorrect. It happens to be the same value when no other user name is specified, otherwise whatever gets resolved is used for the database name (but it is still a "default" because the database name was not explicitly specified).
David J.
On Wed, Jun 1, 2022 at 7:05 AM Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
On 31.05.22 22:12, David G. Johnston wrote:
> Anything I should be doing differently here to get a bit of
> reviewer/committer time on these? I'll add them to the commitfest for
> next month if needed but I'm seeing quick patches going in every week
> and the batch format done at the beginning of the month got processed
> through without issue.
It might have been better to post these
separately.
Doing so hasn't seemed to make a difference. Still not getting even a single committer on a single patch willing to either work with me to get it committed, or decide otherwise, despite my prompt responses on the few comments that have been given.
Given the lack of feedback and my track record I don't think it unreasonable for someone to simply commit these without comment and let any late naysayers voice their opinions after it hits the repo.
In any case, I've added these and more to the commitfest at this point so hopefully the changed mindset of committers when it comes to clearing out the commitfest backlog will come into play in a couple of weeks and I might see some action.
David J.