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

Reply via email to