From: | Marc Cousin <cousinmarc(at)gmail(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Review: rollback sequence reset for TRUNCATE ... RESTART IDENTITY |
Date: | 2010-11-17 13:13:55 |
Message-ID: | 201011171413.55967.cousinmarc@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi, Here is my review of 'rollback sequence reset for TRUNCATE ... RESTART
IDENTITY' patch.
- Is the patch in context diff format?
It's in git diff format. I guess it's OK ?
- Does it apply cleanly to the current git master?
Yes
- Does it include reasonable tests, necessary doc patches, etc?
Doc: Yes, it removes the warning about TRUNCATE ... RESTART IDENTITY, which is
the point of the patch
Tests: There is a new regression test added for restart identity. And 'make
check' passes (tested on linux).
- Usability review (skills needed: test-fu, ability to find and read spec)
- Read what the patch is supposed to do, and consider:
- Does the patch actually implement that?
Yes.
- Do we want that?
I think so, it removes a trap limitation of truncate
- Do we already have it?
No
- Does it follow SQL spec, or the community-agreed behavior?
I think so
- Does it include pg_dump support (if applicable)?
Not applicable
- Are there dangers?
Not that I think of
- Have all the bases been covered?
I think so
- Feature test (skills needed: patch, configure, make, pipe errors to log)
- Apply the patch, compile it and test:
- Does the feature work as advertised?
Yes. It works consistently, isn't fooled by savepoints or multiple serials in
a table, or concurrent transactions
- Are there corner cases the author has failed to consider?
I don't think so
- Are there any assertion failures or crashes?
No
Performance review (skills needed: ability to time performance)
- Does the patch slow down simple tests?
No
- If it claims to improve performance, does it?
Not applicable
- Does it slow down other things?
No
- Read the changes to the code in detail and consider:
- Does it follow the project coding guidelines?
Yes
- Are there portability issues?
I don't think so
- Will it work on Windows/BSD etc?
Yes. There is no OS-specific code added.
- Are the comments sufficient and accurate?
Yes
- Does it do what it says, correctly?
Yes
- Does it produce compiler warnings?
No
- Can you make it crash?
No
- Consider the changes to the code in the context of the project as a whole:
- Is everything done in a way that fits together coherently with other
features/modules?
Yes
- Are there interdependencies that can cause problems?
No
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2010-11-17 13:19:27 | Re: changing MyDatabaseId |
Previous Message | Markus Wanner | 2010-11-17 12:57:18 | Re: changing MyDatabaseId |