You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cayenne.apache.org by "Øyvind Harboe (JIRA)" <ji...@apache.org> on 2010/02/24 09:06:28 UTC

[jira] Created: (CAY-1395) createSetAllowNullToDb() return incorrect SQL statement to createSql()

createSetAllowNullToDb() return incorrect SQL statement to createSql()
----------------------------------------------------------------------

                 Key: CAY-1395
                 URL: https://issues.apache.org/jira/browse/CAY-1395
             Project: Cayenne
          Issue Type: Bug
          Components: Core Library
    Affects Versions: 3.0RC2
            Reporter: Øyvind Harboe
             Fix For: 3.0


The problem with createSql() is that it is executed *before* the property is changed.

The following kludge to SQLServerManagerFactory.createSetAllowNullToDb() works on my rocket...


        return new SetAllowNullToDb(entity, column) {

            @Override
            public List<String> createSql(DbAdapter adapter) {
                StringBuffer sqlBuffer = new StringBuffer();

                QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
                        .getDataMap()
                        .isQuotingSQLIdentifiers());

                sqlBuffer.append("ALTER TABLE ");
                sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
                sqlBuffer.append(" ALTER COLUMN ");

                /* Kludge!!!! how should this be handled really??? */
                column.setMandatory(false);
                adapter.createTableAppendColumn(sqlBuffer, column);

                return Collections.singletonList(sqlBuffer.toString());
            }


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


[jira] Commented: (CAY-1395) createSetAllowNullToDb() return incorrect SQL statement to createSql()

Posted by "Øyvind Harboe (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CAY-1395?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12837765#action_12837765 ] 

Øyvind Harboe commented on CAY-1395:
------------------------------------

Without the kludge above, the SQL statement will be

ALTER xxxx NOT NULL 

instead of

ALTER XXXX NULL

as SetAllowNullToDb() invokes isMandatory() and isMandatory() returns true when converting a DbAttribute to not mandatory.


>From JdbcAdapter.java:


sqlBuffer.append(column.isMandatory() ? " NOT NULL" : " NULL");

> createSetAllowNullToDb() return incorrect SQL statement to createSql()
> ----------------------------------------------------------------------
>
>                 Key: CAY-1395
>                 URL: https://issues.apache.org/jira/browse/CAY-1395
>             Project: Cayenne
>          Issue Type: Bug
>          Components: Core Library
>    Affects Versions: 3.0RC2
>            Reporter: Øyvind Harboe
>             Fix For: 3.0
>
>
> The problem with createSql() is that it is executed *before* the property is changed.
> The following kludge to SQLServerManagerFactory.createSetAllowNullToDb() works on my rocket...
>         return new SetAllowNullToDb(entity, column) {
>             @Override
>             public List<String> createSql(DbAdapter adapter) {
>                 StringBuffer sqlBuffer = new StringBuffer();
>                 QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
>                         .getDataMap()
>                         .isQuotingSQLIdentifiers());
>                 sqlBuffer.append("ALTER TABLE ");
>                 sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
>                 sqlBuffer.append(" ALTER COLUMN ");
>                 /* Kludge!!!! how should this be handled really??? */
>                 column.setMandatory(false);
>                 adapter.createTableAppendColumn(sqlBuffer, column);
>                 return Collections.singletonList(sqlBuffer.toString());
>             }

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


[jira] Updated: (CAY-1395) createSetAllowNullToDb() can lead to data corruption, returns incorrect SQL statement to createSql()

Posted by "Andrus Adamchik (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CAY-1395?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andrus Adamchik updated CAY-1395:
---------------------------------

    Fix Version/s: 3.0.1
                       (was: 3.0-final)

> createSetAllowNullToDb() can lead to data corruption, returns incorrect SQL statement to createSql() 
> -----------------------------------------------------------------------------------------------------
>
>                 Key: CAY-1395
>                 URL: https://issues.apache.org/jira/browse/CAY-1395
>             Project: Cayenne
>          Issue Type: Bug
>          Components: Core Library
>    Affects Versions: 3.0RC2
>            Reporter: Øyvind Harboe
>             Fix For: 3.0.1
>
>
> SQLServerManagerFactory.createSetAllowNullToDb() can cause data corruption.
> The problem is that it assumes that the column passed in to the constructor contains the properties read from the database.
> Consider a text field where the column passed to the constructor is *shorter* than what's currently used in the database.
> In the use of a a merger context one could decide *only* to try to change the allow null property and leave the length of the text field alone.  In this case changing the allow null property will truncate the length of the text field as a side effect.
> Consider the MS SQL server syntax to set the NOT NULL (do not allow null) syntax:
> ALTER TABLE consequence ALTER COLUMN cons_id int NOT NULL
> Here the type of the field must preceed the NOT NULL and hence you have to *know* the type in the database to modify the allow null property while leaving the type alone...
> SetNotNullToDb() does not suffer from this problem as it changes *only* the allow null property.
>     @Override
>     public List<String> createSql(DbAdapter adapter) {
>         StringBuilder sqlBuffer = new StringBuilder();
>         QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
>                 .getDataMap()
>                 .isQuotingSQLIdentifiers());
>         sqlBuffer.append("ALTER TABLE ");
>         sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
>         sqlBuffer.append(" ALTER COLUMN ");
>         sqlBuffer.append(context.quoteString(getColumn().getName()));
>         sqlBuffer.append(" SET NOT NULL");
>         return Collections.singletonList(sqlBuffer.toString());
>     }
> I've hacked up SQLMergerFactory locally and it appears to work fine in my particular case:
>         return new SetAllowNullToDb(entity, column) {
>             @Override
>             public List<String> createSql(DbAdapter adapter) {
>                 StringBuffer sqlBuffer = new StringBuffer();
>                 QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
>                         .getDataMap()
>                         .isQuotingSQLIdentifiers());
>                 sqlBuffer.append("ALTER TABLE ");
>                 sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
>                 sqlBuffer.append(" ALTER COLUMN ");
>                 /* Kludge!!!! how should this be handled really??? */
>                 column.setMandatory(false);
>                 adapter.createTableAppendColumn(sqlBuffer, column);
>                 return Collections.singletonList(sqlBuffer.toString());
>             }

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


[jira] Updated: (CAY-1395) createSetAllowNullToDb() can lead to data corruption, returns incorrect SQL statement to createSql()

Posted by "Øyvind Harboe (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CAY-1395?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Øyvind Harboe updated CAY-1395:
-------------------------------

    Description: 
SQLServerManagerFactory.createSetAllowNullToDb() can cause data corruption.

The problem is that it assumes that the column passed in to the constructor contains the properties read from the database.

Consider a text field where the column passed to the constructor is *shorter* than what's currently used in the database.

In the use of a a merger context one could decide *only* to try to change the allow null property and leave the length of the text field alone.  In this case changing the allow null property will truncate the length of the text field as a side effect.

Consider the MS SQL server syntax to set the NOT NULL (do not allow null) syntax:

ALTER TABLE consequence ALTER COLUMN cons_id int NOT NULL

Here the type of the field must preceed the NOT NULL and hence you have to *know* the type in the database to modify the allow null property while leaving the type alone...

SetNotNullToDb() does not suffer from this problem as it changes *only* the allow null property.

    @Override
    public List<String> createSql(DbAdapter adapter) {
        StringBuilder sqlBuffer = new StringBuilder();
        QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
                .getDataMap()
                .isQuotingSQLIdentifiers());
        sqlBuffer.append("ALTER TABLE ");
        sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
        sqlBuffer.append(" ALTER COLUMN ");
        sqlBuffer.append(context.quoteString(getColumn().getName()));
        sqlBuffer.append(" SET NOT NULL");

        return Collections.singletonList(sqlBuffer.toString());
    }



I've hacked up SQLMergerFactory locally and it appears to work fine in my particular case:




        return new SetAllowNullToDb(entity, column) {

            @Override
            public List<String> createSql(DbAdapter adapter) {
                StringBuffer sqlBuffer = new StringBuffer();

                QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
                        .getDataMap()
                        .isQuotingSQLIdentifiers());

                sqlBuffer.append("ALTER TABLE ");
                sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
                sqlBuffer.append(" ALTER COLUMN ");

                /* Kludge!!!! how should this be handled really??? */
                column.setMandatory(false);
                adapter.createTableAppendColumn(sqlBuffer, column);

                return Collections.singletonList(sqlBuffer.toString());
            }


  was:
The problem with createSql() is that it is executed *before* the property is changed.

The following kludge to SQLServerManagerFactory.createSetAllowNullToDb() works on my rocket...


        return new SetAllowNullToDb(entity, column) {

            @Override
            public List<String> createSql(DbAdapter adapter) {
                StringBuffer sqlBuffer = new StringBuffer();

                QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
                        .getDataMap()
                        .isQuotingSQLIdentifiers());

                sqlBuffer.append("ALTER TABLE ");
                sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
                sqlBuffer.append(" ALTER COLUMN ");

                /* Kludge!!!! how should this be handled really??? */
                column.setMandatory(false);
                adapter.createTableAppendColumn(sqlBuffer, column);

                return Collections.singletonList(sqlBuffer.toString());
            }


        Summary: createSetAllowNullToDb() can lead to data corruption, returns incorrect SQL statement to createSql()   (was: createSetAllowNullToDb() return incorrect SQL statement to createSql())

> createSetAllowNullToDb() can lead to data corruption, returns incorrect SQL statement to createSql() 
> -----------------------------------------------------------------------------------------------------
>
>                 Key: CAY-1395
>                 URL: https://issues.apache.org/jira/browse/CAY-1395
>             Project: Cayenne
>          Issue Type: Bug
>          Components: Core Library
>    Affects Versions: 3.0RC2
>            Reporter: Øyvind Harboe
>             Fix For: 3.0
>
>
> SQLServerManagerFactory.createSetAllowNullToDb() can cause data corruption.
> The problem is that it assumes that the column passed in to the constructor contains the properties read from the database.
> Consider a text field where the column passed to the constructor is *shorter* than what's currently used in the database.
> In the use of a a merger context one could decide *only* to try to change the allow null property and leave the length of the text field alone.  In this case changing the allow null property will truncate the length of the text field as a side effect.
> Consider the MS SQL server syntax to set the NOT NULL (do not allow null) syntax:
> ALTER TABLE consequence ALTER COLUMN cons_id int NOT NULL
> Here the type of the field must preceed the NOT NULL and hence you have to *know* the type in the database to modify the allow null property while leaving the type alone...
> SetNotNullToDb() does not suffer from this problem as it changes *only* the allow null property.
>     @Override
>     public List<String> createSql(DbAdapter adapter) {
>         StringBuilder sqlBuffer = new StringBuilder();
>         QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
>                 .getDataMap()
>                 .isQuotingSQLIdentifiers());
>         sqlBuffer.append("ALTER TABLE ");
>         sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
>         sqlBuffer.append(" ALTER COLUMN ");
>         sqlBuffer.append(context.quoteString(getColumn().getName()));
>         sqlBuffer.append(" SET NOT NULL");
>         return Collections.singletonList(sqlBuffer.toString());
>     }
> I've hacked up SQLMergerFactory locally and it appears to work fine in my particular case:
>         return new SetAllowNullToDb(entity, column) {
>             @Override
>             public List<String> createSql(DbAdapter adapter) {
>                 StringBuffer sqlBuffer = new StringBuffer();
>                 QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
>                         .getDataMap()
>                         .isQuotingSQLIdentifiers());
>                 sqlBuffer.append("ALTER TABLE ");
>                 sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
>                 sqlBuffer.append(" ALTER COLUMN ");
>                 /* Kludge!!!! how should this be handled really??? */
>                 column.setMandatory(false);
>                 adapter.createTableAppendColumn(sqlBuffer, column);
>                 return Collections.singletonList(sqlBuffer.toString());
>             }

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


[jira] Updated: (CAY-1395) createSetAllowNullToDb() can lead to data corruption, returns incorrect SQL statement to createSql()

Posted by "Ari Maniatis (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CAY-1395?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ari Maniatis updated CAY-1395:
------------------------------

    Fix Version/s: 3.1M1
                   3.0.2
                       (was: 3.0.1)

> createSetAllowNullToDb() can lead to data corruption, returns incorrect SQL statement to createSql() 
> -----------------------------------------------------------------------------------------------------
>
>                 Key: CAY-1395
>                 URL: https://issues.apache.org/jira/browse/CAY-1395
>             Project: Cayenne
>          Issue Type: Bug
>          Components: Core Library
>    Affects Versions: 3.0RC2
>            Reporter: Øyvind Harboe
>             Fix For: 3.1M1, 3.0.2
>
>
> SQLServerManagerFactory.createSetAllowNullToDb() can cause data corruption.
> The problem is that it assumes that the column passed in to the constructor contains the properties read from the database.
> Consider a text field where the column passed to the constructor is *shorter* than what's currently used in the database.
> In the use of a a merger context one could decide *only* to try to change the allow null property and leave the length of the text field alone.  In this case changing the allow null property will truncate the length of the text field as a side effect.
> Consider the MS SQL server syntax to set the NOT NULL (do not allow null) syntax:
> ALTER TABLE consequence ALTER COLUMN cons_id int NOT NULL
> Here the type of the field must preceed the NOT NULL and hence you have to *know* the type in the database to modify the allow null property while leaving the type alone...
> SetNotNullToDb() does not suffer from this problem as it changes *only* the allow null property.
>     @Override
>     public List<String> createSql(DbAdapter adapter) {
>         StringBuilder sqlBuffer = new StringBuilder();
>         QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
>                 .getDataMap()
>                 .isQuotingSQLIdentifiers());
>         sqlBuffer.append("ALTER TABLE ");
>         sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
>         sqlBuffer.append(" ALTER COLUMN ");
>         sqlBuffer.append(context.quoteString(getColumn().getName()));
>         sqlBuffer.append(" SET NOT NULL");
>         return Collections.singletonList(sqlBuffer.toString());
>     }
> I've hacked up SQLMergerFactory locally and it appears to work fine in my particular case:
>         return new SetAllowNullToDb(entity, column) {
>             @Override
>             public List<String> createSql(DbAdapter adapter) {
>                 StringBuffer sqlBuffer = new StringBuffer();
>                 QuotingStrategy context = adapter.getQuotingStrategy(getEntity()
>                         .getDataMap()
>                         .isQuotingSQLIdentifiers());
>                 sqlBuffer.append("ALTER TABLE ");
>                 sqlBuffer.append(context.quoteFullyQualifiedName(getEntity()));
>                 sqlBuffer.append(" ALTER COLUMN ");
>                 /* Kludge!!!! how should this be handled really??? */
>                 column.setMandatory(false);
>                 adapter.createTableAppendColumn(sqlBuffer, column);
>                 return Collections.singletonList(sqlBuffer.toString());
>             }

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