Hi Dan,
 
I have addressed your comments with the latest patch attached to the JIRA entry. 
 
In addition, I have also added a master file for the phaseTester test in java\testing\org\apache\derbyTesting\functionTests\master
Output of the manual run of this test can be diffed against the master that I am checking in. In future, if anyone changes this test such that it's output will change, then they should change the master file accordingly. This way, we can make sure that the test is still running correctly as it is evolving. Once this test is run as part of a suite like other tests, we will not have to run this test manually and any changes to master will be caught easily,
 
Also, here is the svn stat output which I forgot to attach to my previous patch emails.
M      java\engine\org\apache\derby\impl\load\Import.java
M      java\engine\org\apache\derby\impl\sql\GenericStatement.java
M      java\engine\org\apache\derby\impl\sql\compile\sqlgrammar.jj
M      java\engine\org\apache\derby\impl\sql\conn\GenericLanguageConnectionContext.java
M      java\engine\org\apache\derby\impl\sql\catalog\DataDictionaryImpl.java
M      java\engine\org\apache\derby\impl\sql\catalog\DD_Version.java
M      java\engine\org\apache\derby\impl\jdbc\metadata.properties
M      java\engine\org\apache\derby\impl\jdbc\EmbedStatement.java
M      java\engine\org\apache\derby\impl\jdbc\EmbedDatabaseMetaData.java
M      java\engine\org\apache\derby\impl\jdbc\EmbedPreparedStatement.java
M      java\engine\org\apache\derby\iapi\sql\Statement.java
M      java\engine\org\apache\derby\iapi\sql\conn\LanguageConnectionContext.java
M      java\engine\org\apache\derby\iapi\sql\dictionary\DataDictionary.java
M      java\engine\org\apache\derby\iapi\reference\SQLState.java
M      java\engine\org\apache\derby\loc\messages_en.properties
M      java\testing\org\apache\derbyTesting\functionTests\tests\lang\db2Compatibility.sql
M      java\testing\org\apache\derbyTesting\functionTests\tests\lang\copyfiles.ant
A      java\testing\org\apache\derbyTesting\functionTests\tests\lang\optimizerOverrides.sql
M      java\testing\org\apache\derbyTesting\functionTests\tests\lang\checkConstraint.sql
D      java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\metadataJdbc20.java
M      java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\metadata_test.java
M      java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\metadata.java
M      java\testing\org\apache\derbyTesting\functionTests\tests\store\access.sql
M      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNet\metadata.out
D      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNet\metadataJdbc20.out
M      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNet\odbc_metadata.out
A      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNet\optimizerOverrides.out
M      java\testing\org\apache\derbyTesting\functionTests\master\db2Compatibility.out
D      java\testing\org\apache\derbyTesting\functionTests\master\metadataJdbc20.out
M      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNetClient\metadata.out
D      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNetClient\metadataJdbc20.out
M      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNetClient\odbc_metadata.out
A      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNetClient\optimizerOverrides.out
D      java\testing\org\apache\derbyTesting\functionTests\master\j9_foundation\metadataJdbc20.out
A      java\testing\org\apache\derbyTesting\functionTests\master\optimizerOverrides.out
M      java\testing\org\apache\derbyTesting\functionTests\master\checkConstraint.out
M      java\testing\org\apache\derbyTesting\functionTests\master\metadata.out
M      java\testing\org\apache\derbyTesting\functionTests\master\access.out
M      java\testing\org\apache\derbyTesting\functionTests\master\odbc_metadata.out
A      java\testing\org\apache\derbyTesting\functionTests\master\phaseTester.out
M      java\testing\org\apache\derbyTesting\functionTests\suites\jdbc20.runall
M      java\testing\org\apache\derbyTesting\functionTests\suites\derbylang.runall
M      java\testing\org\apache\derbyTesting\functionTests\suites\derbynetmats.runall
M      java\testing\org\apache\derbyTesting\upgradeTests\phaseTester.java
M      java\testing\org\apache\derbyTesting\upgradeTests\runphases.ksh
 
 
thanks,
Mamta

 
On 12/8/05, Daniel John Debrunner <[EMAIL PROTECTED]> wrote:
Mamta Satoor wrote:

Thanks for the patch. I looked mainly at the soft/hard upgrade code.

- Good to add comments to the upgrade code you added in
DD_Version.doFullUpgrade. I'd assumed it was incorrectly using
DD_VERSION_THIS_SOFTWARE_VERSION instead of DD_VERSION_DERBY_10_2, but I
think this is the generic upgrade code you mention in the jira comments.
Though why do you need to check the version at this point, the higher
code has decided that a full upgrade is required?

- In EmbedDatabaseMetaData.notInSoftUpgradeMode the comments about 'not
changing the system tables' I think are misleading. I think you mean the
stored versions of the JDBC database meta data queries. To me, changing
the system tables means things like adding a column to a system table.

- DataDictionary.DD_VERSION_THIS_SOFTWARE_VERSION - there was already a
mechanism to check that the database had been upgraded to the current
verion, using checkVersion with DataDictionary.DD_VERSION_CURRENT. Thus
this new field can be removed. (I think).

- The new method LanguageConnectionContext.prepareInternalStatement , is
the fourth parameter really 'forMetaData' or more generically
'allowInternalSyntax'? I've spent time over the last two years trying to
decrease the number of prepare methods in the code, so I'm not thrilled
to see a new one. Especially as it's probably only used for meta-data,
so the last two parameters are fixed 'true, true', thus if you wanted a
prepared for meta-data only method, you could get away with a two
argument one, maybe even one as the schema descriptor should not be
required.

This additional prepare method comes out of the soft upgrade code for
database meta data, I'd like to try and spend some time before Sat
seeing if there's an alternative. I thought there was some code to
already handle this type of case which would mean no changes to
EmbedDatabaseMetaData were required.

I think this patch also addresses DERBY-727, add the upgrade framework.


Dan.



Reply via email to