Bugzilla Version 2.16.7
Bugzilla Bug 1816
  Sequence id generation produces an incorrect sql statement
     Query page      Enter new bug
Bug#: 1816   Platform:   Reporter: francisco dot castaneda at e-vision dot com dot mx
Product:   OS:   Add CC:
Component:   Version:   CC:
Remove selected CCs
Status: CLOSED   Priority:  
Resolution: FIXED   Severity:  
Assigned To: werner dot guttmann at gmx dot net (Werner Guttmann)   Target Milestone:  
URL:
Summary:

Attachment Type Modified Status Actions
Proposed patch for SequenceKeyGenerator.java patch 2004-12-07 16:06 none Edit
proposed patch patch 2004-12-08 11:32 none Edit
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 1816 depends on: Show dependency tree
Show dependency graph
Bug 1816 blocks:
Votes: 0    Show votes for this bug    Vote for this bug

Additional Comments:


Leave as CLOSED FIXED
Reopen bug

View Bug Activity   |   Format For Printing

Description: Opened: 2004-12-06 15:37

Sequence id generation produces an incorrect sql statement

I have the following sequence generator:

    <key-generator name="SEQUENCE" alias="secuencias_basicas">
        <param name="trigger" value="false"/>
        <param name="returning" value="false"/>
        <param name="sequence" value="{0}_SEQ"/>
    </key-generator>

And i'm using it in the following object:

	<class name="mpk.tiempos.entidades.organizacion.Areas" key-
generator="secuencias_basicas" identity="id">
		<map-to table="organizacion.Areas"/>
		<field name="id" type="int">
			<sql name="id" type="integer"/>
		</field>
		<field name="descripcion" type="string" required="true">
			<sql name="descripcion" type="char"/>
		</field>
		<field name="estaActivo" type="boolean" required="true">
			<sql name="estaActivo" type="integer"/>
		</field>
	</class>

When I try to create an object the following sql statement is produced:

INSERT INTO "ORGANIZACION"."AREAS" ("ID","DESCRIPCION","ESTAACTIVO") VALUES 
("ORGANIZACION"".""AREAS_SEQ"."NEXTVAL",?,?)

looking carefully to the statement you will notice 
that "ORGANIZACION"".""AREAS_SEQ"."NEXTVAL" has 2 double quotes around the dot 
sign and of course that is an illegal statement in Oracle. 

This feature used to work in previous releases, and now it only happens when i 
use the <schema>.<table> nomenclature to define tables. It also doesn't happen 
if instead of ={0}_SEQ I use the full name of the sequence (p.e. 
ORGANIZACION.AREAS_SEQ).

I'll try to look at the source code, but since I'm not familiar with it, it'll 
take me some time to fix it, so if someone of the exolab team could take a 
look at this problem I'll really appreciate it because my whole project 
depends on this.

Thanks in advance

------- Additional Comment #1 From francisco dot castaneda at e-vision dot com dot mx 2004-12-06 17:32 -------
I've traced the error down to the 
org.exolab.castor.jdo.drivers.SequenceKeyGenerator class. 

The method public String patchSQL( String insert, String primKeyName ) didn-t 
get rid of every double quote that the table name had, and specifically in the 
case of sequences identified using its full name <schema>.<object> the dot 
sign was enclosed between double quotes. 

I fixed the problem and i tested it against my examples and worked ok, so I'd 
like to post the fix, so How do I do that?

Here is the code

    /**
     * Gives a possibility to patch the Castor-generated SQL statement
     * for INSERT (makes sense for DURING_INSERT key generators)
     */
    public String patchSQL( String insert, String primKeyName )
            throws MappingException {
        StringTokenizer st;
        String tableName;
        String seqName;
        String nextval;
        StringBuffer sb;
        int lp1;  // the first left parenthesis, which starts fields list
        int lp2;  // the second left parenthesis, which starts values list

        if ( _style == BEFORE_INSERT ) {
            return insert;
        }

        // First find the table name
        st = new StringTokenizer( insert );
        if ( !st.hasMoreTokens() || !st.nextToken().equalsIgnoreCase
("INSERT") ) {
            throw new MappingException( Messages.format
( "mapping.keyGenCannotParse",
                                                         insert ) );
        }
        if ( !st.hasMoreTokens() || !st.nextToken().equalsIgnoreCase("INTO") ) 
{
            throw new MappingException( Messages.format
( "mapping.keyGenCannotParse",
                                                         insert ) );
        }
        if ( !st.hasMoreTokens() ) {
            throw new MappingException( Messages.format
( "mapping.keyGenCannotParse",
                                                         insert ) );
        }
        tableName = st.nextToken();

		/**
		 * This method didnīt remove every double quote so it was 
changed.
		 */
        // remove quotes
/*        if ( tableName.startsWith("\"") ) {
            tableName = tableName.substring( 1, tableName.length() - 1 );
        }*/

		/**
		 * This procedure removes every double quote in the tablename.
		 */

		StringBuffer buffer2 = new StringBuffer(tableName);
		
		for (int index=buffer2.indexOf("\"");index 
>=0;index=buffer2.indexOf("\"")){
			buffer2.deleteCharAt(index);	
		}
			
		tableName = buffer2.toString();

        seqName = MessageFormat.format( _seqName, new String[] 
{tableName,primKeyName});
        nextval = _factory.quoteName(seqName + ".nextval");
        lp1 = insert.indexOf( '(' );
        lp2 = insert.indexOf( '(', lp1 + 1 );
        if ( lp1 < 0 ) {
            throw new MappingException( Messages.format
( "mapping.keyGenCannotParse",
                                                         insert ) );
        }
        sb = new StringBuffer( insert );
        if (!_triggerPresent) { // if no onInsert triggers in the DB, we have 
to supply the Key values manually
           if ( lp2 < 0 ) {
                // Only one pk field in the table, the INSERT statement would 
be
                // INSERT INTO table VALUES ()
                lp2 = lp1;
                lp1 = insert.indexOf( " VALUES " );
                // don't change the order of lines below,
                // otherwise index becomes invalid
                sb.insert( lp2 + 1, nextval);
                sb.insert( lp1 + 1, "(" + _factory.quoteName( primKeyName ) 
+ ") " );
            } else {
                // don't change the order of lines below,
                // otherwise index becomes invalid
                sb.insert( lp2 + 1, nextval + ",");
                sb.insert( lp1 + 1, _factory.quoteName( primKeyName ) + "," );
            }
        }
        if ( _style == DURING_INSERT ) {
            // append 'RETURNING primKeyName INTO ?'
            sb.append( " RETURNING " );
            sb.append( _factory.quoteName( primKeyName ) );
            sb.append( " INTO ?" );
        }
        return sb.toString();
    }




------- Additional Comment #2 From Werner Guttmann 2004-12-07 01:24 -------
Hola Francisco, can you please post your patch in unified format ? 
Running 'diff -u' normally does the job alright.

------- Additional Comment #3 From francisco dot castaneda at e-vision dot com dot mx 2004-12-07 16:06 -------
Created an attachment (id=812)
Proposed patch for SequenceKeyGenerator.java 

diff -u -r1.5 SequenceKeyGenerator.java

------- Additional Comment #4 From francisco dot castaneda at e-vision dot com dot mx 2004-12-07 16:09 -------
Hello Werner,

I hope this is what's needed, if not please give me a hand and provide me with 
further instructions.

Mis mejores saludos 

------- Additional Comment #5 From Werner Guttmann 2004-12-08 02:44 -------
Muchas gracias, Franscisco. Eso 'patch' es valido.

Martin, just wondering whether this is a similar problem to what we are trying
to resolve in bug 1793? (postgreSQL specific) ?

------- Additional Comment #6 From Werner Guttmann 2004-12-08 05:58 -------
I actually meant bug 1317, but never mind .. ;-). It is a similar problem, indeed.

------- Additional Comment #7 From Werner Guttmann 2004-12-08 06:07 -------
Actually, whilst the patch is a valid patch, it's not a unified patch against
the CVS repository. Apparently, you created a second copy of
SequenceKeyGenerator in a directory named b, and diffed it against
a/SequenceKeyGenerator.

What I'd need is a unified patch against the latest version of the code in the
CVS repository. Would you be willing the get the latest sources from the CVS
server, make the change in your local code and run the diff command to create a
unified patch between your local changes and the CVS code repository.

I know that I may be asking a lot (and it definitely is not really required for
a patch of this size), but it'd ease my life in general - working with a lot of
patches at this time of the year.

------- Additional Comment #8 From Martin Fuchs 2004-12-08 11:32 -------
Created an attachment (id=814)
proposed patch

It's not the same problem as that in bug 1317, but is also about double quotes.
:)
This patch is a somewhat optimized version of Francisco's bug fix.

------- Additional Comment #9 From francisco dot castaneda at e-vision dot com dot mx 2004-12-09 16:47 -------
Werner, Martin,

I didn't have the chance to submit my diff patch, but today I read Martin's 
mail and then I guess the patch that Martin sent closes the bug, Am I right?

Then, Can I expect to see this as an add on to version 9.5.4?

How does this work?

Best regards

Francisco



------- Additional Comment #10 From Werner Guttmann 2004-12-11 01:59 -------
Francisco, I'll commit Martin's patch in due time (i.e. read some when this
weekend), but I am afraid you'll have to wait for 0.9.6 for this to surface in
an official release. 0.9.5.4 has been released already, and unless there are any
urgent regression issues with features/bugs introduced in 0.9.5.4, there won't
be any enhancements to this release any more.

------- Additional Comment #11 From Werner Guttmann 2004-12-11 02:26 -------
Patch committed as supplied. Please don't forget to close this bug once you've
assessed its correctness.

     Query page      Enter new bug
This is Bugzilla: the Mozilla bug system. For more information about what Bugzilla is and what it can do, see bugzilla.org.
Actions: New | Query | bug # | Reports   New Account | Log In