Changes

Jump to: navigation, search

PostgreSQL Adapter Project - Code Review 4 Changes

1,342 bytes added, 16:06, 20 September 2011
no edit summary
|getQuotedName
* functionality is not needed just use SQLSchemamanager.quote() the way this is done in other sql manager code
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/72 DONE''']
|}
*remove the condition
*it should be possible to set 0 timeout
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/73 DONE''']
|-
|CancelTask -> StatementCancelationTask;
*make it a top level class
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/74 DONE''']
|}
*change the comment to "Reads a column from a JDBC driver result set and adds it to a table object".
*Move rs argument to the last position
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/75 DONE'''|-|I think the portable flag should not take into account columns that are not added to the table (return null in this case)?|'''DONE''']
|-
| The message about ignoring an index has changed
*remove the separator flag.
*Adding separators should be done in different code. Each method should do as little as possible.
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/77 DONE''']
|-
|isPortable()
*(Methods should not try to do several tasks at once, in this case skipping invalid entries and determining portability.
This hampers method reusability.).
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/76 DONE''']
|-
| isWindowsCompatiable -> isWindowsCompatible
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/78 DONE''']
|}
|*Proxy.java -> *Wrapper.java
*(this is not the proxy pattern, but more like the decorator pattern)
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/79 DONE''']
|}
*The comment is incorrect. This is not 1048576*8*10 and is not 1GB, but 10MB. Remove "~~"
*MAX_LO_SIZE - Remove "~~" from the comment. This is 2GB-1.
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/80 DONE''']
|-
|BIND_BLOB
*If an error occurs, lo will not be closed. Please fix this.
* setAutoCommit - why is this modified at all? We cannot simply change the autocommit flag on a connection.
|''[https://bitbucket.org/gbatumbya/postgresql_external/issue/87 DONE''']
*Remove BIND_BLOB
|-
*should be moved to the cancelation task, as static methods.
*The timer singleton instance (lazy initialized) should be there as well.
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/81 DONE''']
|-
|unwrap
|appendIdentitySuffix
*Add a space after ";" for readability
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/82 DONE''']
|-
|appendLiteral
*TIMESTAMP_ORDINAL - this should be in UTC, I think that simply converting the timestamp to a string might not do it.
*Try to see if using Primitive.toString() will provide the right format.
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/83 DONE''']
*used StringUtil.appendUTC
|-
|appendTypeConversion
*TIMESTAMP_ORDINAL - please verify that this actually formats/parses UTC timestamps.
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/84 DONE''']
*modified postgresql setup scripts to set timezone to UTC
*changed to use date_part('epoch', timestamp)
|indexNameMatches
*Spaces around commas are needed. Wrap the line after about 110 chars.
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/85 DONE''']
|-
|isUnicode
*catch(SQLException e) space before (
*Do not repackage the SQLException as a runtime exception - the method already allows throwing an SQLException
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/86 DONE''']
|}
*Use visibility declarators on members ("protected")
*Fold lines after about 110 chars
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/88 DONE''']
|-
|requestSavepoint
* I think not, since the savepoints are only created inside a transaction block
* No, the driver does not allow setSavePoint on XA transaction (org.postgresql.xa.PGXAConnection)
*'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/89 DONE'''*'''DONE''']
|}
*Use getColumnAlterationToken
*Are booleans indexable in Postgres? If not, we should use integers instead.
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/90 DONE''']
*Yes, it supports indexing booleans. Suggested use is with partial indexes on the condition that queries will use most often.
|-
*Method JavaDoc comments are required.
*What does this do?
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/91 DONE''']
|-
|appendTSIncrement
*interval' – perhaps a space before "'" for readability?
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/92 DONE''']
|-
|getGUIDExpr
*Remove "XXX: from the comment
*Move the comment to the class header comment
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/93 DONE''']
|-
|isImplicitConversion
*srcType == Primitive.BOOLEAN ) - remove space before )
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/94 DONE''']
|-
|isValidColumnName
*Spaces around "-"
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/95 DONE''']
|-
|addColumn
*This might be worth refactoring further by adding a template method in order to avoid code duplication with the base class.
|[https://bitbucket.org/gbatumbya/postgresql_external/issue/97 DONE]
|-
|{
;
} undo the change to ;
|'''[https://bitbucket.org/gbatumbya/postgresql_external/issue/96 DONE''']
|}
1
edit

Navigation menu