db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mamta Satoor <msat...@gmail.com>
Subject Re: [PATCH](DERBY-783) Enhance ALTER TABLE syntax to allow users to change the next value to be generated for an identity column
Date Wed, 25 Jan 2006 16:34:43 GMT
Hi Satheesh,

I have made changes so the autoIncrementStart in SYSCOLUMNS gets set to new
value provided by RESTART WITH. I have attached the patch to the JIRA entry.

svn stat
M
java\engine\org\apache\derby\impl\sql\catalog\SYSCOLUMNSRowFactory.java
M
java\testing\org\apache\derbyTesting\functionTests\master\autoincrement.out

thanks,
Mamta


On 1/23/06, Mamta Satoor <msatoor@gmail.com> wrote:
>
> I can look into it to see if it would impact anything if the
> autoIncrementStart is changed in SYSCOLUMNS. If everything looks good, I
> will post a patch soon.
>
> thanks,
> Mamta
>
>
>  On 1/23/06, Satheesh Bandaram <satheesh@sourcery.org> wrote:
> >
> > Hi Mamta,
> >
> > Thanks for addressing the comments...
> >
> > Mamta Satoor wrote:
> >
> > Hi Satheesh,
> >
> > 2)I didn't consider changing autoIncrementStart in
> > SYSCOLUMNSRowFactory.java because this value doesn't get looked at when
> > the next value is generated after the RESTART WITH alter table command.
> >
> > Right... This start value is not used for generating the next value...
> > but if any applications look at the START value from the catalogs for any
> > reason, it would still show OLD start value, not the new value set by
> > RESTART option. Do you see any problems in setting START with the new value?
> > It is documented in the manuals, under catalogs.
> >
> > Satheesh
> >
> >
> >
> > On 1/20/06, Satheesh Bandaram <satheesh@sourcery.org > wrote:
> > >
> > > Hi Mamta,
> > >
> > > I think we need to mark 'RESTART' as a non-reserved word and also have
> > > couple of other comments. This can be submitted separately, so I committed
> > > the patch.
> > >
> > > Transmitting file data ..............
> > > Committed revision 370885.
> > >
> > > Here are some comments:
> > >
> > >    1. RESTART needs to be marked as non-reserved word. Can you add
> > >    this to nonReservedKeyword() list? Currently, attempts to create a table
> > >    with RESTART name fails. (considers it as reserved word)
> > >    2. About your change to SYSCOLUMNSRowFactory.java, should we
> > >    mark autoIncrementStart with the new value?
> > >    3. +               {//user asked for restart with a new value,
> > >    so don't change increment by and original start
> > >    +                       //with values in the SYSCOLUMNS table.
> > >    Just record the RESTART WITH value as the
> > >    +                       //next value to be generated in the
> > >    SYSCOLUMNS table
> > >    +                       ColumnDescriptor  column =
> > >    (ColumnDescriptor)td;
> > >    +                       row.setColumn(SYSCOLUMNS_AUTOINCREMENTVALUE,
> > >    new SQLLongint(autoincStart));
> > >    *+                       row.setColumn(SYSCOLUMNS_AUTOINCREMENTSTART,
> > >    new SQLLongint(*
> > >    *+
> > >    column.getTableDescriptor
> > >    ().getColumnDescriptor(colName).getAutoincStart()));*
> > >    +                       row.setColumn(SYSCOLUMNS_AUTOINCREMENTINC,
> > >    new SQLLongint(
> > >    +
> > >    column.getTableDescriptor
> > >    ().getColumnDescriptor(colName).getAutoincInc()));
> > >    4. Is there a need to prevent RESTART option under soft upgrade?
> > >    I don't think so, since you are not adding any disk footprint, but just
> > >    thought I will bring it up.
> > >
> > > Satheesh
> > >
> > > Mamta Satoor wrote:
> > >
> > > Hi Satheesh,
> > >
> > > I have rerun the tests and didn't see any failures after doing a sync.
> > > Can you please look into commiting the new review package attached to the
> > > JIRA entry?
> > >
> > > thanks,
> > > Mamta
> > >
> > >
> > > On 1/16/06, Mamta Satoor <msatoor@gmail.com> wrote:
> > > >
> > > > Hi Satheesh,
> > > >
> > > > Thanks for looking into committing this patch. I have done a sync on
> > > > my client and will look into resolving the conflicts. After that, I want
to
> > > > rerun all the tests before submitting an updated patch.
> > > >
> > > > Mamta
> > > >
> > > >  On 1/16/06, Satheesh Bandaram <satheesh@sourcery.org > wrote:
> > > > >
> > > > > Hi Mamta,
> > > > >
> > > > > Since I applied your other patch, this patch can't be applied as
> > > > > is. (since they share some of the same files) Can you update your
patch and
> > > > > post it, please?
> > > > >
> > > > > Satheesh
> > > > >
> > > > > Mamta Satoor wrote:
> > > > >
> > > > > Posted this few days back. Anyone has comments?
> > > > >
> > > > > Thanks,
> > > > > Mamta
> > > > >
> > > > >
> > > > > On 1/3/06, Mamta Satoor <msatoor@gmail.com> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I have attached a review package for this feature to the JIRA
> > > > > > entry. Following is a brief description of the changes involved.
> > > > > >
> > > > > > Changed sqlgrammar.jj to add parser support for ALTER TABLE
> > > > > > <tablename> ALTER <columnName> RESTART WITH integer-constant
> > > > > > Also, added another element to the array which keeps track of
> > > > > > autoincrement information in the parser. This 4th element will
record if the
> > > > > > autoincrement column is getting added or it is getting altered
for INCREMENT
> > > > > > BY value change or it is getting altered for RESTART WITH value
> > > > > > change. This information is required later in the compile and
execute phase.
> > > > > >
> > > > > >
> > > > > > In the compile phase, this information is used to see if a user
> > > > > > is trying to sneak in a value of 0 for INCREMENT BY. A value
of 0 for
> > > > > > INCREMENT BY should be caught at the time of autoincrement column
add or at
> > > > > > the time of autoincrement column alter to change the INCREMENT
> > > > > > BY value. At the time of autoincrement column alter to change
> > > > > > the RESTART WITH value, the INCREMENT BY value should not be
> > > > > > checked. This is done in ColumnDefinitionNode.java.
> > > > > > TableElementList generates ColumnInfo which needs to keep track
of
> > > > > > autoincrement column change status from ColumnDefinitionNode.
This
> > > > > > infromation in ColumnInfo will be used at execute time.
> > > > > >
> > > > > > In the execute phase, we need to know which columns of
> > > > > > SYSCOLUMNS table need to be changed for an ALTER TABLE command
> > > > > > on the autoincrement column. In the past, we only allowed to
change the
> > > > > > INCREMENT BY criteria of an autoincrement column but with this
> > > > > > feature, it is possible for a user to change the start withvalue
of autoincrement column and leave the INCREMENT BY unchanged. This
> > > > > > autoincrement column change information is passed to the execute
phase via
> > > > > > ColumnInfo.
> > > > > >
> > > > > > In order to provide this distinction between ALTER BY..INCREMENT
> > > > > > BY.. and ALTER BY..RESTART WITH.., I have had to add a variable
> > > > > > in ColumnDefinitionNode.java, ColumnInfo.java and
> > > > > > ColumnDescriptor.java. The value of the variable in each of
> > > > > > these classes depend on what parser recorded for the autoincrement
column
> > > > > > status ie adding an autoincrement column/changing INCREMENT
BY of the
> > > > > > autoincrement column/changing RESTART WITH of the autoincrement
> > > > > > column.
> > > > > >
> > > > > > Hope this information along with the comments in the code will
> > > > > > help in the code review. Please let me know if you have any
comments.
> > > > > >
> > > > > > The svn stat changes for this feature is as follows
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\compile\QueryTreeNode.java
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\compile\ColumnDefinitionNode.java
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\compile\CreateSchemaNode.java
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\compile\sqlgrammar.jj
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\compile\TableElementList.java
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\compile\CreateViewNode.java
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\compile\ModifyColumnNode.java
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\execute\ColumnInfo.java
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\execute\CreateTableConstantAction.java
> > > > > >
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\execute\AlterTableConstantAction.java
> > > > > > M
> > > > > > java\engine\org\apache\derby\impl\sql\catalog\SYSCOLUMNSRowFactory.java
> > > > > > M
> > > > > > java\engine\org\apache\derby\iapi\sql\dictionary\ColumnDescriptor.java
> > > > > > M
> > > > > > java\testing\org\apache\derbyTesting\functionTests\tests\lang\autoincrement.sql
> > > > > > M
> > > > > > java\testing\org\apache\derbyTesting\functionTests\master\autoincrement.out
> > > > > > I have modified autoincrement.sql to add tests for RESTART WITH.
> > > > > > The derbyall suite ran fine on my Windows XP machine with
> > > > > > Sun's jdk142.
> > > > > >
> > > > > > thanks,
> > > > > > Mamta
> > > > > >
> > > > >
> > > > >
> > > >
> > >
> >
>

Mime
View raw message