openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Dick (JIRA)" <>
Subject [jira] Commented: (OPENJPA-946) Oracle create table(s) exceptions
Date Wed, 13 May 2009 14:08:45 GMT


Michael Dick commented on OPENJPA-946:

Some comments on the patch.

The method name getReservedColumnNames() is misleading. It's really returning the the set
of reserved words - reserved words that can be used as column names. Renaming to getValidColumnNames()
makes more sense to me. Javadoc would also help here - I had to go back to the declaration
of the instance variable to figure out why the method was correct. 

In addition this method recreates the list each time it's invoked - it'd be better to either
cache the list after the first invocation or create the set when the dictionary is initialized.

I'm not sure I have a good solution at hand, but it looks like a lot of the dictionaries reuse
the same set of allowable column names. It'd be nice if we could store that list in a single
place instead of potentially maintaining it in each class. 

Otherwise the patch looks good, thanks very much for looking into it Tim. The testcase is
very nice and appreciated. 

> Oracle create table(s) exceptions
> ---------------------------------
>                 Key: OPENJPA-946
>                 URL:
>             Project: OpenJPA
>          Issue Type: Sub-task
>            Reporter: Tim McConnell
>            Assignee: Tim McConnell
>         Attachments: OPENJPA-946-3.patch

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message