db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Army <qoz...@sbcglobal.net>
Subject Re: Patch again again for DERBY-167.
Date Tue, 17 May 2005 00:50:48 GMT
TomohitoNakayama wrote:
> Hello.
> 
> I send new patch for DERBY-167 , which is attached to this mail.
> Please review it.

Thanks for the sending the new patch.

I've reviewed it and I have the following comments.  The first one is an 
important one, the second one is very minor.

First:

The Derby "dblook" utility will have to be modified to account for the "BY 
DEFAULT" keyword--right now, it generates all autoincrement columns using the 
"ALWAYS" keyword.  See org/apache/derby/impl/tools/dblook/DB_Table.java, in the 
"reinsateAutoIncrement" method.  Luckily, this change should be fairly easy 
given the changes in your patch.

Since your patch sets the "DefaultInfo" for a BY DEFAULT column to a non-null 
value, I think dblook can just check the "COLUMNDEFAULT" column in the 
SYS.SYSCOLUMNS table.  If it is _not_ null, then dblook can generate "BY 
DEFAULT" instead of "ALWAYS".

For example, you could change the "getAutoIncStmt" statement at the top of 
DB_Table.java to retrieve the COLUMNDEFAULT column:

getAutoIncStmt =
	conn.prepareStatement("SELECT AUTOINCREMENTSTART, " +
	"AUTOINCREMENTINC, COLUMNDEFAULT FROM SYS.SYSCOLUMNS " +
	"WHERE COLUMNNAME = ? AND REFERENCEID = ?");

and then change the "reinstateAutoIncrement(...)" method to do the following:

.
.
.
if (autoIncCols.next()) {
	long start = autoIncCols.getLong(1);
	if (!autoIncCols.wasNull()) {
		colDef.append(" GENERATED ");
		// -- begin new logic --
		String def = autoIncCols.getString(3);
		colDef.append(autoIncCols.wasNull() ? "ALWAYS" : "BY DEFAULT");
		// -- end new logic --
		colDef.append(" AS IDENTITY (START WITH ");
		colDef.append(autoIncCols.getLong(1));
		colDef.append(", INCREMENT BY ");
		colDef.append(autoIncCols.getLong(2));
		colDef.append(")");
	}
}

This will make it so that dblook generates "BY DEFAULT" correctly for columns 
that require it.

And then it'd probably be good to add a case for this in 
java/testing/org/apache/derbyTesting/tests/tools/dblook_makeDB.sql, which is the 
script that is used for testing dblook.  In that script, there's a section under 
the "Tables" heading that indicates "auto increment/default" test cases.  You 
could add a simple table for the "GENERATED BY DEFAULT" case.  If you have any 
problems getting that to work, feel free to post your questions and I'll try to 
help where I can.

Second (minor):

I noticed that in DefaultInfoImpl.java, the two new methods 
(getDefinitionBitsValue and calcIsDefaultValueAutoinc) are declared as 
static--is there a reason for that?  I made them non-static and everything 
compiled, so I'm just wondering if this was intentional?

Thanks again for re-sending the patch, and please feel free to ask if you have 
any questions about I've written here.

Army


Mime
View raw message