openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kevin Sutter (JIRA)" <j...@apache.org>
Subject [jira] Commented: (OPENJPA-399) openjpa did not handle multiple schema names with same table name
Date Fri, 12 Oct 2007 03:29:50 GMT

    [ https://issues.apache.org/jira/browse/OPENJPA-399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12534211
] 

Kevin Sutter commented on OPENJPA-399:
--------------------------------------

Comments on the patches...  Overall, the concept and logic looks good.  I just have a few
questions/comments about the proposed code changes.  Ping me if you have any questions on
my abbreviated comments.  Thanks.

o  The restriction about requiring schema names for all tables if you use schema names for
at least one of the entities using the same table name...  I'm okay with this restriction,
but is there anyway that we could detect this condition?  That is, if we detect the use of
a "null" schema with other existing schema names, couldn't we flag this a configuration error?

o  Any updated patches should be attached to the Issue with the same name.  JIRA keeps track
of the older versions automatically, so you don't have to continually add a "version number"
to your patch files.

o  The testcases should be provided as part of the patch file.  Makes it much easier to merge
the new files into an existing project.  It also removes the confusion with different project
names (ie. openjpa10x_2 doesn't match my project name).

o  The testcases need the Apache licensing...  :-)  Take a look, you'll understand.

o  In TableJDBCSeq, the getStatus method updates are interesting.  So, the previous implementation
of this method didn't act on the input parameter at all?  That's strange.  Also, the javadoc
for this method indicates that the input parameter (mapping) could be null.  Is it okay for
your code to use "null" as a key for the HashMap?

o  In that same file, why not just initialize _stat to the new HashMap() (similar to the original
"new Status()")?  And, the original declaration was final.  If you use an initializer, couldn't
we go back to the "final" declaration?  Maybe I'm missing the reasons for your current _stat
initialization.

o  In the addSchema() method, there's no longer a "fast path" return conditional at the beginning
of the method.  Isn't there any means of bypassing all of the processing if the table/schema
already exists?

o  I'm not following your code at the end of the addSchema() method with the "Index idx" processing.
 It looks like this index processing is a side-effect of calling addSchema().  Is that your
intent?  At a minimum, this requires additional comments, but maybe it requires some re-factoring
to make this clearer.

o  The processing of generating the tableName is repeated in at least three areas of TableJDBCSeq.
 Could a common utility method be used instead so as not to repeat the code?

o  I don't understand the comments for Column.resetTableName().  Doesn't "reset" mean to modify
an existing set, yet the comment indicates that this can only be called on columns without
a table set.

o  The cleanup in LocalConstraint.addColumn() looks good.

o  The logic in SchemaGroup.findTable and SchemaGroup.findSequence seems to be very close.
 Can any of this code be shared in a private utility method?

That's it,
Kevin

> openjpa did not handle multiple schema names with same table name
> -----------------------------------------------------------------
>
>                 Key: OPENJPA-399
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-399
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: jdbc
>    Affects Versions: 1.0.1
>         Environment: JDK1.5, OPENJPA verison 580425
>            Reporter: Teresa Kan
>            Assignee: Teresa Kan
>         Attachments: OPENJPA_399.patch, OPENJPA_399_2.patch, TestMultipleSchemaNames.zip
>
>
> Two entities have the same table name but with different schema, only one table is created.
In addition, when two entities use the generatedType.AUTO for ID, only one OPENJPA_SEQUENCE-TABLE
is created.
> The problem due to the SchemaGroup.findTable() which only looked for a table name from
all the schemas. Once the table was found in one of the schema then it exited and assumed
that the table existed. Same problem in the TableJDBCSeq.addSchema().

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


Mime
View raw message