Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.4.2.1
    • Fix Version/s: 10.6.1.0
    • Component/s: Test
    • Labels:
      None
    • Urgency:
      Normal
    • Issue & fix info:
      Newcomer

      Description

      Converting altertable.sql harness test to JUnit

      1. AlterTableTest.java
        110 kB
        Bryan Pendleton
      2. AlterTableTest.java
        117 kB
        Bryan Pendleton
      3. AlterTableTest.java
        117 kB
        Bryan Pendleton
      4. ASF.LICENSE.NOT.GRANTED--AlterTable.diff
        177 kB
        Eranda Sooriyabandara
      5. ASF.LICENSE.NOT.GRANTED--AlterTable.java
        288 kB
        Eranda Sooriyabandara
      6. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        275 kB
        Eranda Sooriyabandara
      7. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        150 kB
        Eranda Sooriyabandara
      8. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        144 kB
        Eranda Sooriyabandara
      9. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        160 kB
        Eranda Sooriyabandara
      10. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        114 kB
        Eranda Sooriyabandara
      11. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        121 kB
        Eranda Sooriyabandara
      12. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        121 kB
        Eranda Sooriyabandara
      13. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        124 kB
        Eranda Sooriyabandara
      14. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        130 kB
        Eranda Sooriyabandara
      15. ASF.LICENSE.NOT.GRANTED--AlterTableTest.diff
        138 kB
        Eranda Sooriyabandara
      16. reformatPatch.diff
        143 kB
        Bryan Pendleton
      17. removeFromOldSuite.diff
        267 kB
        Bryan Pendleton
      18. svn_stat.txt
        0.6 kB
        Bryan Pendleton

        Activity

        Hide
        Eranda Sooriyabandara added a comment -

        I convert altertable.sql to AlterTable.junit under Bryan Pendletons
        supervision and I attach both AlterTable.java and AlterTable.diff files
        here.

        Show
        Eranda Sooriyabandara added a comment - I convert altertable.sql to AlterTable.junit under Bryan Pendletons supervision and I attach both AlterTable.java and AlterTable.diff files here.
        Hide
        Knut Anders Hatlen added a comment -

        Hi Eranda,

        I haven't performed a complete review of your patch, but I have some high-level comments:

        • The AlterTable class is in package stack, whereas it should be in package org.apache.derbyTesting.functionTests.tests.lang
        • By convention, we normally add a Test suffix to the class names when we write JUnit tests, so the class name of the new test would be AlterTableTest
        • There are some parts of the test that haven't been converted, marked with [**:: UNCONVERTED ::**]. I think we should try to find a way to convert those parts before we disable the old test
        • the old test used to set some properties, see derbyTesting/functionTests/tests/lang/altertable_derby.properties
        • altertable.sql is removed, but the old regression test suite still attempts to run it, so it should be removed from derbyTesting/functionTests/suites/derbylang.runall too. Also, the files functionTests/master/altertable.out and functionTests/tests/lang/altertable_derby.properties should be removed when the test is removed
        Show
        Knut Anders Hatlen added a comment - Hi Eranda, I haven't performed a complete review of your patch, but I have some high-level comments: The new file needs an Apache license header. http://www.apache.org/legal/src-headers.html The AlterTable class is in package stack, whereas it should be in package org.apache.derbyTesting.functionTests.tests.lang By convention, we normally add a Test suffix to the class names when we write JUnit tests, so the class name of the new test would be AlterTableTest There are some parts of the test that haven't been converted, marked with [**:: UNCONVERTED ::**] . I think we should try to find a way to convert those parts before we disable the old test the old test used to set some properties, see derbyTesting/functionTests/tests/lang/altertable_derby.properties altertable.sql is removed, but the old regression test suite still attempts to run it, so it should be removed from derbyTesting/functionTests/suites/derbylang.runall too. Also, the files functionTests/master/altertable.out and functionTests/tests/lang/altertable_derby.properties should be removed when the test is removed
        Hide
        Eranda Sooriyabandara added a comment -

        Hi knut,
        Thanks for corrections on making a patch.
        As a beginning I did,

        1.AlterTable.java -->AlterTableTest.java
        2. add Apache license header to the java file
        3.remove derbyTesting/functionTests/tests/lang/altertable_derby.properties
        4.altertable.sql is removed from the old regression test suite

        Can you please make a comment on how it could be done unconverted parts
        converted.

        Show
        Eranda Sooriyabandara added a comment - Hi knut, Thanks for corrections on making a patch. As a beginning I did, 1.AlterTable.java -->AlterTableTest.java 2. add Apache license header to the java file 3.remove derbyTesting/functionTests/tests/lang/altertable_derby.properties 4.altertable.sql is removed from the old regression test suite Can you please make a comment on how it could be done unconverted parts converted.
        Hide
        Knut Anders Hatlen added a comment -

        To convert the unconverted parts, I think you'll need to find some way to do the same thing as the IJ commands via JDBC, possibly using DatabaseMetaData.

        For instance this code

        + /*[**:: UNCONVERTED ::**] describe renc_vw_1

        +

        + [**:: UNCONVERTED ::**] COLUMN_NAME |TYPE_NAME|DEC&|NUM&|COLUM&|COLUMN_DEF|CHAR_OCTE&|IS_NULL&

        +------------------------------------------------------------------------------

        +V1 |VARCHAR |NULL|NULL|10 |NULL |20 |YES

        +V2 |DOUBLE |NULL|2 |52 |NULL |NULL |YES

        +

        + */

        could perhaps be replaced with a call to DatabaseMetaData.getColumns() and verify that the table has the expected columns.

        I guess that those looking like this could be removed:

        + [**:: UNCONVERTED ::**] Issue the 'help' command for general information on IJ command syntax.

        +Any unrecognized commands are treated as potential SQL commands and executed directly.

        +Consult your DBMS server reference documentation for details of the SQL syntax supported by your server.

        +

        + */

        Show
        Knut Anders Hatlen added a comment - To convert the unconverted parts, I think you'll need to find some way to do the same thing as the IJ commands via JDBC, possibly using DatabaseMetaData. For instance this code + /* [**:: UNCONVERTED ::**] describe renc_vw_1 + + [**:: UNCONVERTED ::**] COLUMN_NAME |TYPE_NAME|DEC&|NUM&|COLUM&|COLUMN_DEF|CHAR_OCTE&|IS_NULL& +------------------------------------------------------------------------------ +V1 |VARCHAR |NULL|NULL|10 |NULL |20 |YES +V2 |DOUBLE |NULL|2 |52 |NULL |NULL |YES + + */ could perhaps be replaced with a call to DatabaseMetaData.getColumns() and verify that the table has the expected columns. I guess that those looking like this could be removed: + [**:: UNCONVERTED ::**] Issue the 'help' command for general information on IJ command syntax. +Any unrecognized commands are treated as potential SQL commands and executed directly. +Consult your DBMS server reference documentation for details of the SQL syntax supported by your server. + + */
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Knut,
        Instead of having one GIANT test in AlterTableTest.java, if
        we had about 5-6 individual test cases, as in

        testAddColumn()
        testAddConstraint()
        testDropColumn()
        testRenameColumn()
        testAlterIdentityColumn()

        and so forth will be better. If we separate them in to individual test
        how can we
        identify whether which part of code is in which test.
        Thanks
        Eranda

        Show
        Eranda Sooriyabandara added a comment - Hi Knut, Instead of having one GIANT test in AlterTableTest.java, if we had about 5-6 individual test cases, as in testAddColumn() testAddConstraint() testDropColumn() testRenameColumn() testAlterIdentityColumn() and so forth will be better. If we separate them in to individual test how can we identify whether which part of code is in which test. Thanks Eranda
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Knut,
        I replace unconverted cord as you say by
        DatabaseMetaData.getColumns(). Though it didn't show compile errors(on
        DatabaseMetaData.getColumns()) in the the netbeans IDE when compile
        using ant it shows 21 compile errors in it.

        As I search for DatabaseMetadata.getColumns() method it shows that,
        -public ResultSet getColumns(String catalog,String
        schemaPattern,String tableNamePattern,String columnNamePattern) throws
        -SQLException
        has 4 parameters and we did not use them. Can it be the causing of error?

        Here I attach the changed code.
        Thanks
        Eranda

        Show
        Eranda Sooriyabandara added a comment - Hi Knut, I replace unconverted cord as you say by DatabaseMetaData.getColumns(). Though it didn't show compile errors(on DatabaseMetaData.getColumns()) in the the netbeans IDE when compile using ant it shows 21 compile errors in it. As I search for DatabaseMetadata.getColumns() method it shows that, -public ResultSet getColumns(String catalog,String schemaPattern,String tableNamePattern,String columnNamePattern) throws -SQLException has 4 parameters and we did not use them. Can it be the causing of error? Here I attach the changed code. Thanks Eranda
        Hide
        Bryan Pendleton added a comment -

        Hi Eranda, thanks for the updated test.

        Here's the section of section of altertable.out where the conversion program was having
        problems, because it doesn't know how to convert the "describe" and "show indexes"
        statements.

        In this code, I think that the "describe" and "show indexes" statements were present only
        in order to show that the view and the index were not changed by the statements which
        tried to rename the column in the view or the index.

        I think that we can just remove these unconverted sections; it is sufficient just to test
        for the syntax error on the RENAME statements which reference the view and the index.

        ij> rename column renc_vw_1.v2 to v3;
        ERROR 42Y62: 'RENAME COLUMN' is not allowed on '"APP"."RENC_VW_1"' because it is a view.
        ij> describe renc_vw_1;
        COLUMN_NAME |TYPE_NAME|DEC&|NUM&|COLUM&|COLUMN_DEF|CHAR_OCTE&|IS_NULL&
        ------------------------------------------------------------------------------
        V1 |VARCHAR |NULL|NULL|10 |NULL |20 |YES
        V2 |DOUBLE |NULL|2 |52 |NULL |NULL |YES
        ij> – attempt to rename a column in an index, should fail:
        create index renc_idx_1 on renc_1 (c, d);
        0 rows inserted/updated/deleted
        ij> show indexes from renc_1;
        TABLE_NAME |COLUMN_NAME |NON_U&|TYPE|ASC&|CARDINA&|PAGES
        ----------------------------------------------------------------------------
        RENC_1 |C |true |3 |A |NULL |NULL
        RENC_1 |D |true |3 |A |NULL |NULL
        ij> rename column renc_idx_1.d to d_new;
        ERROR 42Y55: 'RENAME COLUMN' cannot be performed on 'RENC_IDX_1' because it does not exist.
        ij> show indexes from renc_1;
        TABLE_NAME |COLUMN_NAME |NON_U&|TYPE|ASC&|CARDINA&|PAGES
        ----------------------------------------------------------------------------
        RENC_1 |C |true |3 |A |NULL |NULL
        RENC_1 |D |true |3 |A |NULL |NULL

        Show
        Bryan Pendleton added a comment - Hi Eranda, thanks for the updated test. Here's the section of section of altertable.out where the conversion program was having problems, because it doesn't know how to convert the "describe" and "show indexes" statements. In this code, I think that the "describe" and "show indexes" statements were present only in order to show that the view and the index were not changed by the statements which tried to rename the column in the view or the index. I think that we can just remove these unconverted sections; it is sufficient just to test for the syntax error on the RENAME statements which reference the view and the index. ij> rename column renc_vw_1.v2 to v3; ERROR 42Y62: 'RENAME COLUMN' is not allowed on '"APP"."RENC_VW_1"' because it is a view. ij> describe renc_vw_1; COLUMN_NAME |TYPE_NAME|DEC&|NUM&|COLUM&|COLUMN_DEF|CHAR_OCTE&|IS_NULL& ------------------------------------------------------------------------------ V1 |VARCHAR |NULL|NULL|10 |NULL |20 |YES V2 |DOUBLE |NULL|2 |52 |NULL |NULL |YES ij> – attempt to rename a column in an index, should fail: create index renc_idx_1 on renc_1 (c, d); 0 rows inserted/updated/deleted ij> show indexes from renc_1; TABLE_NAME |COLUMN_NAME |NON_U&|TYPE|ASC&|CARDINA&|PAGES ---------------------------------------------------------------------------- RENC_1 |C |true |3 |A |NULL |NULL RENC_1 |D |true |3 |A |NULL |NULL ij> rename column renc_idx_1.d to d_new; ERROR 42Y55: 'RENAME COLUMN' cannot be performed on 'RENC_IDX_1' because it does not exist. ij> show indexes from renc_1; TABLE_NAME |COLUMN_NAME |NON_U&|TYPE|ASC&|CARDINA&|PAGES ---------------------------------------------------------------------------- RENC_1 |C |true |3 |A |NULL |NULL RENC_1 |D |true |3 |A |NULL |NULL
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan/Knut,
        I tested some of the function include in AlterTableTest.java after
        remove unconverted cords.
        Here are my testings,
        ij> connect 'jdbc:derby:Movies;create=true';
        ij> CREATE TABLE Persons ( P_Id int, LastName varchar(255), FirstName
        varchar(255), Address varchar(
        255), City varchar(255) );
        0 rows inserted/updated/deleted
        ij> rename table Persons to Studentdetails;
        0 rows inserted/updated/deleted
        ij> ALTER TABLE Studentdetails DROP LastName;
        0 rows inserted/updated/deleted
        ij> select *from Studentdetails;
        P_ID |FIRSTNAME |ADDRESS |CITY
        ----------------------------------------------------------------------------------------------------
        0 rows selected
        ij> ALTER TABLE Studentdetails ADD COLUMN LastName varchar(255);
        0 rows inserted/updated/deleted
        ij> select *from Studentdetails;
        P_ID |FIRSTNAME |ADDRESS |CITY |L ASTNAME
        ----------------------------------------------------------------------------------------------------
        0 rows selected
        ij> ALTER TABLE Studentdetails CHANGE address address1 varchar(255);
        ERROR 42X01: Syntax error: Encountered "CHANGE" at line 1, column 28.
        Issue the 'help' command for general information on IJ command syntax.
        Any unrecognized commands are treated as potential SQL commands and
        executed directly.
        Consult your DBMS server reference documentation for details of the
        SQL syntax supported by your ser
        ver.
        After testing I can say that, in alter table
        Functions like Rename table, Drop column, Add column, Add constraint,
        Drop constraint are working properly but “Change Column name” does
        not.
        Thanks
        Eranda

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan/Knut, I tested some of the function include in AlterTableTest.java after remove unconverted cords. Here are my testings, ij> connect 'jdbc:derby:Movies;create=true'; ij> CREATE TABLE Persons ( P_Id int, LastName varchar(255), FirstName varchar(255), Address varchar( 255), City varchar(255) ); 0 rows inserted/updated/deleted ij> rename table Persons to Studentdetails; 0 rows inserted/updated/deleted ij> ALTER TABLE Studentdetails DROP LastName; 0 rows inserted/updated/deleted ij> select *from Studentdetails; P_ID |FIRSTNAME |ADDRESS |CITY ---------------------------------------------------------------------------------------------------- 0 rows selected ij> ALTER TABLE Studentdetails ADD COLUMN LastName varchar(255); 0 rows inserted/updated/deleted ij> select *from Studentdetails; P_ID |FIRSTNAME |ADDRESS |CITY |L ASTNAME ---------------------------------------------------------------------------------------------------- 0 rows selected ij> ALTER TABLE Studentdetails CHANGE address address1 varchar(255); ERROR 42X01: Syntax error: Encountered "CHANGE" at line 1, column 28. Issue the 'help' command for general information on IJ command syntax. Any unrecognized commands are treated as potential SQL commands and executed directly. Consult your DBMS server reference documentation for details of the SQL syntax supported by your ser ver. After testing I can say that, in alter table Functions like Rename table, Drop column, Add column, Add constraint, Drop constraint are working properly but “Change Column name” does not. Thanks Eranda
        Hide
        Bryan Pendleton added a comment -

        Hi Eranda,

        That is correct. To rename a column you have to use the RENAME COLUMN statement:
        http://db.apache.org/derby/docs/10.5/ref/rrefsqljrenamecolumnstatement.html

        So you could do:
        RENAME COLUMN Studentdetails.address to address1;

        Show
        Bryan Pendleton added a comment - Hi Eranda, That is correct. To rename a column you have to use the RENAME COLUMN statement: http://db.apache.org/derby/docs/10.5/ref/rrefsqljrenamecolumnstatement.html So you could do: RENAME COLUMN Studentdetails.address to address1;
        Hide
        Eranda Sooriyabandara added a comment -

        hi Bryan,

        I attach the patch here.
        Here the message I get as the out put while I run the
        AlterTableTest.java with 2 failures.

        D:\Repositories\trunk>java junit.textui.TestRunner
        org.apache.derbyTesting.functionTests.tests.lang.
        AlterTableTest
        .F.F
        Time: 11.952
        There were 2 failures:
        1) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A
        ssertionFailedError: Expected warning but found none
        at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl
        eTest.java:77)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        2) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A
        ssertionFailedError: Expected error(s) ' X0X95' but no error was thrown.
        at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java
        :976)
        at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java
        :1002)
        at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl
        eTest.java:206)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)

        FAILURES!!!
        Tests run: 2, Failures: 2, Errors: 0

        Thanks
        Eranda

        Show
        Eranda Sooriyabandara added a comment - hi Bryan, I attach the patch here. Here the message I get as the out put while I run the AlterTableTest.java with 2 failures. D:\Repositories\trunk>java junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.lang. AlterTableTest .F.F Time: 11.952 There were 2 failures: 1) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A ssertionFailedError: Expected warning but found none at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl eTest.java:77) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) 2) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A ssertionFailedError: Expected error(s) ' X0X95' but no error was thrown. at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java :976) at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java :1002) at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl eTest.java:206) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) FAILURES!!! Tests run: 2, Failures: 2, Errors: 0 Thanks Eranda
        Hide
        Bryan Pendleton added a comment -

        Hi Eranda, thank for the updated AlterTableTest patch.

        It seems that you are making very good progress.

        I took your patch, and I made some changes to AlterTableTest.java,
        and am attaching the version that I modified.

        There are going to be a number of small aspects to consider.

        Perhaps you can have a look at the AlterTableTest.java that I attached,
        and diff it against yours, and see some of the changes that I made?

        Some changes are because the automatic conversion program does
        not get everything exactly right, and so we have to fix up the test program
        slightly, such as when it gets the column names in the result set wrong.

        Some other changes are because there are behavior differences between
        embedded mode and client/server mode, and we'll have to study those
        each in detail to understand them (such as the case where the ALTER TABLE
        is supposed to behave differently when a cursor is open on the table).

        Please have a look at the revised AlterTableTest that I attached and let me
        know your reaction to the changes that I made.

        If the changes make sense to you, perhaps you can continue in this mode,
        making additional changes as necessary similar to the ones I made.

        Show
        Bryan Pendleton added a comment - Hi Eranda, thank for the updated AlterTableTest patch. It seems that you are making very good progress. I took your patch, and I made some changes to AlterTableTest.java, and am attaching the version that I modified. There are going to be a number of small aspects to consider. Perhaps you can have a look at the AlterTableTest.java that I attached, and diff it against yours, and see some of the changes that I made? Some changes are because the automatic conversion program does not get everything exactly right, and so we have to fix up the test program slightly, such as when it gets the column names in the result set wrong. Some other changes are because there are behavior differences between embedded mode and client/server mode, and we'll have to study those each in detail to understand them (such as the case where the ALTER TABLE is supposed to behave differently when a cursor is open on the table). Please have a look at the revised AlterTableTest that I attached and let me know your reaction to the changes that I made. If the changes make sense to you, perhaps you can continue in this mode, making additional changes as necessary similar to the ones I made.
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,
        Here I have the test result for updated AlterTableTest.java
        D:\Repositories\trunk>java junit.textui.TestRunner
        org.apache.derbyTesting.functionTests.tests.lang.
        AlterTableTest
        .F.F
        Time: 20.083
        There were 2 failures:
        1) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.C
        omparisonFailure: Column names do not match: expected:<[&]> but was:<[TYPE]>
        at org.apache.derbyTesting.junit.JDBC.assertColumnNames(JDBC.java:681)
        at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl
        eTest.java:2460)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        2) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A
        ssertionFailedError: Unexpected column count: expected:<2> but was:<1>
        at org.apache.derbyTesting.junit.JDBC.assertColumnNames(JDBC.java:676)
        at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl
        eTest.java:282)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)

        FAILURES!!!
        Tests run: 2, Failures: 2, Errors: 0

        As it shows that the 1st fail occurs in 2460th row. So I look in to
        the code & in the altertable.out I found that there are two coloumns
        with the same name,
        CONSTRAINTID |TABLEID |CONSTRAINTNAME |&|SCHEMAID |&|REFERENCEC&
        -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
        xxxxFILTERED-UUIDxxxx|xxxxFILTERED-UUIDxxxx|ATDC_CONSTRAINT_1
        & and &.
        Is it possible to have two columns that have same name in single table?
        What is this Error says that, expected:<[&]> but was:<[TYPE]> How can
        I get through that?
        Thanks
        Eranda

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, Here I have the test result for updated AlterTableTest.java D:\Repositories\trunk>java junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.lang. AlterTableTest .F.F Time: 20.083 There were 2 failures: 1) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.C omparisonFailure: Column names do not match: expected:< [&] > but was:< [TYPE] > at org.apache.derbyTesting.junit.JDBC.assertColumnNames(JDBC.java:681) at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl eTest.java:2460) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) 2) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A ssertionFailedError: Unexpected column count: expected:<2> but was:<1> at org.apache.derbyTesting.junit.JDBC.assertColumnNames(JDBC.java:676) at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl eTest.java:282) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) FAILURES!!! Tests run: 2, Failures: 2, Errors: 0 As it shows that the 1st fail occurs in 2460th row. So I look in to the code & in the altertable.out I found that there are two coloumns with the same name, CONSTRAINTID |TABLEID |CONSTRAINTNAME |&|SCHEMAID |&|REFERENCEC& ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- xxxxFILTERED-UUIDxxxx|xxxxFILTERED-UUIDxxxx|ATDC_CONSTRAINT_1 & and &. Is it possible to have two columns that have same name in single table? What is this Error says that, expected:< [&] > but was:< [TYPE] > How can I get through that? Thanks Eranda
        Hide
        Bryan Pendleton added a comment -

        > What is this Error says that, expected:<[&]> but was:<[TYPE]>

        Hi Eranda. The '&' symbol is actually NOT the name of the column. Rather, it
        is a result of running the old test in "ij". The "ij" tool has some special rules about
        column names, and if the column name is a reserved word, "ij" prints "&" instead
        of printing the column name. And if the column name is long, ij truncates the column
        name and prints "&" at the point where it did the truncation.

        You can see the actual column names in this case by looking in the Derby
        reference manual. Here, we can see in altertable.out that we have just done a
        'select * from sys.sysconstraints', so we just need to look up SYSCONSTRAINTS
        in the reference manual at:
        http://db.apache.org/derby/docs/10.5/ref/rrefsistabs23241.html
        and there you can see that the actual column names are:
        CONSTRAINTID, TABLEID, CONSTRAINTNAME, TYPE, SCHEMAID, STATE, REFERENCECOUNT

        So to fix this, all you need to do is to change the ALterTableTest.java at line 2460 to
        use the correct column names instead of the names with the '&' in them.

        There will be a number of places in AlterTableTest.java where you have to make this
        change, but it's pretty mechanical, so hopefully once you see how to make the change
        in one location, you can make the similar change throughout the test program.

        Show
        Bryan Pendleton added a comment - > What is this Error says that, expected:< [&] > but was:< [TYPE] > Hi Eranda. The '&' symbol is actually NOT the name of the column. Rather, it is a result of running the old test in "ij". The "ij" tool has some special rules about column names, and if the column name is a reserved word, "ij" prints "&" instead of printing the column name. And if the column name is long, ij truncates the column name and prints "&" at the point where it did the truncation. You can see the actual column names in this case by looking in the Derby reference manual. Here, we can see in altertable.out that we have just done a 'select * from sys.sysconstraints', so we just need to look up SYSCONSTRAINTS in the reference manual at: http://db.apache.org/derby/docs/10.5/ref/rrefsistabs23241.html and there you can see that the actual column names are: CONSTRAINTID, TABLEID, CONSTRAINTNAME, TYPE, SCHEMAID, STATE, REFERENCECOUNT So to fix this, all you need to do is to change the ALterTableTest.java at line 2460 to use the correct column names instead of the names with the '&' in them. There will be a number of places in AlterTableTest.java where you have to make this change, but it's pretty mechanical, so hopefully once you see how to make the change in one location, you can make the similar change throughout the test program.
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,

        Here I post the AlterTableTest.java row 277-290

        // select * prepared statements do see added columns after alter table

        rs = pSt.executeQuery();
        expColNames = new String []

        {"C1", "C2"}

        ;
        JDBC.assertColumnNames(rs, expColNames);

        expRS = new String [][]
        {

        {"1", null}

        ,

        {"2", null}

        };

        JDBC.assertFullResultSet(rs, expRS, true);

        It gives a failure ,

        test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A
        ssertionFailedError: Unexpected column count: expected:<2> but was:<1>
        at org.apache.derbyTesting.junit.JDBC.assertColumnNames(JDBC.java:676)
        at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl
        eTest.java:282).

        I change the code to,

        // select * prepared statements do "not" see added columns after alter table

        rs = pSt.executeQuery();
        expColNames = new String []

        {"C1"}

        ;
        JDBC.assertColumnNames(rs, expColNames);

        expRS = new String [][]
        {

        {"1"}

        ,

        {"2"}

        };

        JDBC.assertFullResultSet(rs, expRS, true);

        as a testing.

        Then the Failure in that row is not there when I was testing.
        It means that <select * prepared statements do "not" see added columns
        after alter table>
        or another failure in some where else that causing to the result change.

        But some how altertable.out shows two columns,

        ij> – select * prepared statements do see added columns after alter table
        execute p1;
        C1 |C2
        -----------------------
        1 |NULL
        2 |NULL

        Thanks
        Eranda

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, Here I post the AlterTableTest.java row 277-290 // select * prepared statements do see added columns after alter table rs = pSt.executeQuery(); expColNames = new String [] {"C1", "C2"} ; JDBC.assertColumnNames(rs, expColNames); expRS = new String [][] { {"1", null} , {"2", null} }; JDBC.assertFullResultSet(rs, expRS, true); It gives a failure , test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A ssertionFailedError: Unexpected column count: expected:<2> but was:<1> at org.apache.derbyTesting.junit.JDBC.assertColumnNames(JDBC.java:676) at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl eTest.java:282). I change the code to, // select * prepared statements do "not" see added columns after alter table rs = pSt.executeQuery(); expColNames = new String [] {"C1"} ; JDBC.assertColumnNames(rs, expColNames); expRS = new String [][] { {"1"} , {"2"} }; JDBC.assertFullResultSet(rs, expRS, true); as a testing. Then the Failure in that row is not there when I was testing. It means that <select * prepared statements do "not" see added columns after alter table> or another failure in some where else that causing to the result change. But some how altertable.out shows two columns, ij> – select * prepared statements do see added columns after alter table execute p1; C1 |C2 ----------------------- 1 |NULL 2 |NULL Thanks Eranda
        Hide
        Bryan Pendleton added a comment -

        Hi Eranda,

        I think this is interesting. I think that perhaps this was a mistake in the current altertable.sql test.

        The sequence of operations is:
        1) Prepare ("select * from t2")
        2) Execute prepared statement, sees only column c1
        3) alter table t2 add column c2
        4) Execute prepared statement, STILL sees only column c1, though the IJ version saw both c1 AND c2

        I think this is OK behavior. I'm not really sure why there is a behavior difference
        between the IJ-based test and the pure JDBC Junit test, but I think it's acceptable
        for Derby to have either behavior.

        Does anybody else have a theory as to why we see this behavior change when we
        convert this test from an IJ script to a JUnit JDBC test?

        For the time being, I think you should leave AlterTableTest checking for JUST column c1,
        put a comment into the test indicating that when this test is run in IJ, the behavior is
        different, and let's move on to resolve any other problems in the test.

        Show
        Bryan Pendleton added a comment - Hi Eranda, I think this is interesting. I think that perhaps this was a mistake in the current altertable.sql test. The sequence of operations is: 1) Prepare ("select * from t2") 2) Execute prepared statement, sees only column c1 3) alter table t2 add column c2 4) Execute prepared statement, STILL sees only column c1, though the IJ version saw both c1 AND c2 I think this is OK behavior. I'm not really sure why there is a behavior difference between the IJ-based test and the pure JDBC Junit test, but I think it's acceptable for Derby to have either behavior. Does anybody else have a theory as to why we see this behavior change when we convert this test from an IJ script to a JUnit JDBC test? For the time being, I think you should leave AlterTableTest checking for JUST column c1, put a comment into the test indicating that when this test is run in IJ, the behavior is different, and let's move on to resolve any other problems in the test.
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,
        Here I attaching the patch up to now.

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, Here I attaching the patch up to now.
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,

        Thanks for your motivation and I am glad to say that now It's one
        error on the test. Here's results displayed.

        D:\Repositories\trunk>java junit.textui.TestRunner
        org.apache.derbyTesting.functionTests.tests.lang.
        AlterTableTest
        ..F
        Time: 36.547
        There was 1 failure:
        1) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A
        ssertionFailedError: Expected error(s) ' X0X95' but no error was thrown.
        at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java
        :976)
        at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java
        :1002)
        at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl
        eTest.java:2117)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
        at junit.extensions.TestSetup.run(TestSetup.java:27)

        FAILURES!!!
        Tests run: 2, Failures: 1, Errors: 0
        Here I am attaching the patch with this.

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, Thanks for your motivation and I am glad to say that now It's one error on the test. Here's results displayed. D:\Repositories\trunk>java junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.lang. AlterTableTest ..F Time: 36.547 There was 1 failure: 1) test_altertable(org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest)junit.framework.A ssertionFailedError: Expected error(s) ' X0X95' but no error was thrown. at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java :976) at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java :1002) at org.apache.derbyTesting.functionTests.tests.lang.AlterTableTest.test_altertable(AlterTabl eTest.java:2117) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:23) at junit.extensions.TestSetup.run(TestSetup.java:27) FAILURES!!! Tests run: 2, Failures: 1, Errors: 0 Here I am attaching the patch with this.
        Hide
        Bryan Pendleton added a comment -

        Hi Eranda, thanks for the updated test. I made two additional changes:
        1) Added "if (usingEmbedded())" in front of line 2117, and
        2) Changed JDBC.assertFullResultSet to JDBC.assertUnorderedResultSet on line 3036
        and then the test ran successfully for me!

        I've attached AlterTableTest.java with my modifications; can you confirm that
        the test now passes for you?

        Show
        Bryan Pendleton added a comment - Hi Eranda, thanks for the updated test. I made two additional changes: 1) Added "if (usingEmbedded())" in front of line 2117, and 2) Changed JDBC.assertFullResultSet to JDBC.assertUnorderedResultSet on line 3036 and then the test ran successfully for me! I've attached AlterTableTest.java with my modifications; can you confirm that the test now passes for you?
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,
        Wow it passes,Thanks. Can we move on to break test_altertable() in to
        several methods or
        are there something else remains to do with it?
        Thanks,
        Eranda

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, Wow it passes,Thanks. Can we move on to break test_altertable() in to several methods or are there something else remains to do with it? Thanks, Eranda
        Hide
        Bryan Pendleton added a comment -

        Yes, I agree: a good next step is to divide the single large test method into several smaller, more-focused
        test methods, and group the tests into the test methods accordingly.

        Show
        Bryan Pendleton added a comment - Yes, I agree: a good next step is to divide the single large test method into several smaller, more-focused test methods, and group the tests into the test methods accordingly.
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,
        I make some changes to the code in methods testWithSchema() and
        testDropColumn()
        It removes all the Failures in the code and there are 6 errors still there.
        Here I attach the AlterTableTest.diff with this.
        Thanks

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, I make some changes to the code in methods testWithSchema() and testDropColumn() It removes all the Failures in the code and there are 6 errors still there. Here I attach the AlterTableTest.diff with this. Thanks
        Hide
        Bryan Pendleton added a comment -

        Hi Eranda, I have attached a revised copy of AlterTableTest.java.

        It contains a workaround for DERBY-4244, as well as several other
        adjustments and tweaks to the tests. I think the test should be
        passing now; can you confirm this in your environment?

        Show
        Bryan Pendleton added a comment - Hi Eranda, I have attached a revised copy of AlterTableTest.java. It contains a workaround for DERBY-4244 , as well as several other adjustments and tweaks to the tests. I think the test should be passing now; can you confirm this in your environment?
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,
        Thanks for your fixing. The test is also pass in my environment.
        It's really great to see a bug free test.

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, Thanks for your fixing. The test is also pass in my environment. It's really great to see a bug free test.
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,
        Here I attach the complete patch for your concern.
        Thanks

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, Here I attach the complete patch for your concern. Thanks
        Hide
        Bryan Pendleton added a comment -

        Thanks Eranda! I will have a look at the patch as soon as possible.

        Show
        Bryan Pendleton added a comment - Thanks Eranda! I will have a look at the patch as soon as possible.
        Hide
        Bryan Pendleton added a comment -

        Hi Eranda,

        I reformatted the patch somewhat to fix whitespace problems and
        line length problems, and to make the patch easier to read.

        Can you have a look at the latest patch ("reformatPatch.diff") and
        let me know if you see any additional whitespace or formatting
        problems that we need to clean up?

        Show
        Bryan Pendleton added a comment - Hi Eranda, I reformatted the patch somewhat to fix whitespace problems and line length problems, and to make the patch easier to read. Can you have a look at the latest patch ("reformatPatch.diff") and let me know if you see any additional whitespace or formatting problems that we need to clean up?
        Hide
        Bryan Pendleton added a comment -

        Attached patch 'removeFromOldSuite.diff' contains several additional
        changes, to remove references to the old lang/altertable.sql from
        README.htm, from suites/derbylang.runall, and to remove altertable.out.

        My derbyall and junitreport runs were otherwise clean.

        I'll make one more pass over the new test to proof read it and look
        for additional whitespace and formatting cleanup prior to commit.

        Show
        Bryan Pendleton added a comment - Attached patch 'removeFromOldSuite.diff' contains several additional changes, to remove references to the old lang/altertable.sql from README.htm, from suites/derbylang.runall, and to remove altertable.out. My derbyall and junitreport runs were otherwise clean. I'll make one more pass over the new test to proof read it and look for additional whitespace and formatting cleanup prior to commit.
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,
        I complete the comparing altertable.out and AlterTableTest and I found
        some places where they do not match(asserting) but I think most of
        them are doesn't wanted to test for alter table
        functionalities.Except,

        1)
        in line 1378 and 1390 in testAlterColumn()
        the underlying test was not there. I think they must be there to
        confirm that column was renamed and constraint was there after the
        renaming.
        ij> describe renc_2;
        COLUMN_NAME |TYPE_NAME|DEC&|NUM&|COLUM&|COLUMN_DEF|CHAR_OCTE&|IS_NULL&
        ------------------------------------------------------------------------------
        C2 |INTEGER |0 |10 |10 |NULL |NULL |NO
        ij> show indexes from renc_2;
        TABLE_NAME |COLUMN_NAME |NON_U&|TYPE|ASC&|CARDINA&|PAGES
        ----------------------------------------------------------------------------
        RENC_2 |C2 |false |3 |A |NULL |NULL

        If you agree with me I can add the asserting test to the test or we
        can leave it as it is now.
        ----------------------------------------------------------------------------------

        2)
        in line 2019 in testDropColumn() it says that

        ij> – Verify that a GRANTED privilege fails a drop column in RESTRICT mode,
        – and verify that the privilege is dropped in CASCADE mode:

        but according to the test I found that

        st.executeUpdate("alter table atdc_10 drop column b restrict");

        So I change the comment to
        // Verify that a GRANTED privilege fails a drop column in
        // CASCADE mode, and verify that the privilege is dropped
        // in RESTRICT mode:
        But here drop column in cascade mode is not tested. Please make a
        comment that I am correct or not.
        -------------------------------------------------------------------------------------

        3)
        At line 2461 and 2465 in testJira3355()

        st.executeUpdate("create table d3355 ( c1 varchar(10), \"c2\" "
        +"varchar(10), c3 varchar(10))");

        According to the create table statement there is no column named "C3"
        in the table d3355 but it passes as follows
        st.executeUpdate("alter table d3355 alter column \"C3\" not null");

        st.executeUpdate(" create table \"d3355_a\" ( c1 varchar(10), \"c2\" "
        +"varchar(10), c3 varchar(10))");

        According to the create table statement there is no column named "C3"
        in the table "d3355_a" but it passes as follows
        st.executeUpdate("alter table \"d3355_a\" alter column \"C3\" not null");

        This is a contradictory behavior as I think.
        -----------------------------------------------------------------------------------

        Also I add some comment according to the altertable.out as I think its
        appropriate.
        I have gone through the full code and I didn't see any other mistakes
        on the test.
        And here I attach the patch file that I updated.

        Thanks
        Eranda

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, I complete the comparing altertable.out and AlterTableTest and I found some places where they do not match(asserting) but I think most of them are doesn't wanted to test for alter table functionalities.Except, 1) in line 1378 and 1390 in testAlterColumn() the underlying test was not there. I think they must be there to confirm that column was renamed and constraint was there after the renaming. ij> describe renc_2; COLUMN_NAME |TYPE_NAME|DEC&|NUM&|COLUM&|COLUMN_DEF|CHAR_OCTE&|IS_NULL& ------------------------------------------------------------------------------ C2 |INTEGER |0 |10 |10 |NULL |NULL |NO ij> show indexes from renc_2; TABLE_NAME |COLUMN_NAME |NON_U&|TYPE|ASC&|CARDINA&|PAGES ---------------------------------------------------------------------------- RENC_2 |C2 |false |3 |A |NULL |NULL If you agree with me I can add the asserting test to the test or we can leave it as it is now. ---------------------------------------------------------------------------------- 2) in line 2019 in testDropColumn() it says that ij> – Verify that a GRANTED privilege fails a drop column in RESTRICT mode, – and verify that the privilege is dropped in CASCADE mode: but according to the test I found that st.executeUpdate("alter table atdc_10 drop column b restrict"); So I change the comment to // Verify that a GRANTED privilege fails a drop column in // CASCADE mode, and verify that the privilege is dropped // in RESTRICT mode: But here drop column in cascade mode is not tested. Please make a comment that I am correct or not. ------------------------------------------------------------------------------------- 3) At line 2461 and 2465 in testJira3355() st.executeUpdate("create table d3355 ( c1 varchar(10), \"c2\" " +"varchar(10), c3 varchar(10))"); According to the create table statement there is no column named "C3" in the table d3355 but it passes as follows st.executeUpdate("alter table d3355 alter column \"C3\" not null"); st.executeUpdate(" create table \"d3355_a\" ( c1 varchar(10), \"c2\" " +"varchar(10), c3 varchar(10))"); According to the create table statement there is no column named "C3" in the table "d3355_a" but it passes as follows st.executeUpdate("alter table \"d3355_a\" alter column \"C3\" not null"); This is a contradictory behavior as I think. ----------------------------------------------------------------------------------- Also I add some comment according to the altertable.out as I think its appropriate. I have gone through the full code and I didn't see any other mistakes on the test. And here I attach the patch file that I updated. Thanks Eranda
        Hide
        Bryan Pendleton added a comment -

        Hi Eranda, thanks for all the hard work on this test. I think it's starting to look really good.

        Regarding the points you raised above, here's my opinion:
        1) I think it would be a good idea to improve the verification of the contents of
        the system catalogs for the column and the constraint, as you suggest. You
        could have a look at the DatabaseMetadata API; I think it should have most of
        what you need:
        http://java.sun.com/j2se/1.4.2/docs/api/java/sql/DatabaseMetaData.html

        2) The comment is incorrect here. ALTER TABLE DROP COLUMN does not
        care whether or not there are granted privileges on the column, and the presence
        of RESTRICT or CASCADE does not matter in this case. The Derby development
        team discussed this back in 2006 and when I did the DERBY-1909 work, I should
        have put a better comment in the test. Here's a pointer to the 2006 discussion:
        https://issues.apache.org/jira/browse/DERBY-1489?focusedCommentId=12421187&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_12421187

        So I think you should change the comment in the test program to something like:

        // ALTER TABLE DROP COLUMN automatically drops any granted privilege,
        // regardless of whether RESTRICT or CASCADE was specified. Verify that
        // the privileges are dropped correctly and that the bitmap is updated:

        3) I think the test is fine here. I think the confusing part is that this part of the
        test is verifying behavior of both lower-case and upper-case column names. If
        a column or table name is not enclosed in quotation marks, then it is
        automatically converted to upper case, so in fact the column C3 does exist,
        even though the CREATE TABLE statement specified the column name c3
        in lower case; it was automatically converted to upper case, and so this part
        of the test appears to be working correctly to me.

        I think that once we clean up these last few issues, we should be ready to
        perform a final review and commit of this patch. Thanks again for all the hard work!

        Show
        Bryan Pendleton added a comment - Hi Eranda, thanks for all the hard work on this test. I think it's starting to look really good. Regarding the points you raised above, here's my opinion: 1) I think it would be a good idea to improve the verification of the contents of the system catalogs for the column and the constraint, as you suggest. You could have a look at the DatabaseMetadata API; I think it should have most of what you need: http://java.sun.com/j2se/1.4.2/docs/api/java/sql/DatabaseMetaData.html 2) The comment is incorrect here. ALTER TABLE DROP COLUMN does not care whether or not there are granted privileges on the column, and the presence of RESTRICT or CASCADE does not matter in this case. The Derby development team discussed this back in 2006 and when I did the DERBY-1909 work, I should have put a better comment in the test. Here's a pointer to the 2006 discussion: https://issues.apache.org/jira/browse/DERBY-1489?focusedCommentId=12421187&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_12421187 So I think you should change the comment in the test program to something like: // ALTER TABLE DROP COLUMN automatically drops any granted privilege, // regardless of whether RESTRICT or CASCADE was specified. Verify that // the privileges are dropped correctly and that the bitmap is updated: 3) I think the test is fine here. I think the confusing part is that this part of the test is verifying behavior of both lower-case and upper-case column names. If a column or table name is not enclosed in quotation marks, then it is automatically converted to upper case, so in fact the column C3 does exist, even though the CREATE TABLE statement specified the column name c3 in lower case; it was automatically converted to upper case, and so this part of the test appears to be working correctly to me. I think that once we clean up these last few issues, we should be ready to perform a final review and commit of this patch. Thanks again for all the hard work!
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,
        Here I am attaching the patch.

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, Here I am attaching the patch.
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,
        Here I attach the final patch.
        Thanks
        Eranda

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan, Here I attach the final patch. Thanks Eranda
        Hide
        Bryan Pendleton added a comment -

        Hi Eranda, thanks for the updated patch. I will have a look this weekend.

        Show
        Bryan Pendleton added a comment - Hi Eranda, thanks for the updated patch. I will have a look this weekend.
        Hide
        Bryan Pendleton added a comment -

        The new test looks ready to me. My regression runs were clean. The new test is now run
        as part of the lang._Suite in JUnit, and the old test is removed from derbylang. Everything
        seems complete to me, so I've committed this patch to the trunk as revision 791027.

        Eranda, thanks for all the hard work on this conversion. This was a large and complex
        test, and was a tough one to start with; this is very good work. I'm looking forward to
        some more test conversions this summer!

        Please verify that the updated code in the trunk looks correct in your sandbox after
        an 'svn update', and verify that you can build and run the new test from the trunk in
        your environment, and then please mark this issue closed.

        Show
        Bryan Pendleton added a comment - The new test looks ready to me. My regression runs were clean. The new test is now run as part of the lang._Suite in JUnit, and the old test is removed from derbylang. Everything seems complete to me, so I've committed this patch to the trunk as revision 791027. Eranda, thanks for all the hard work on this conversion. This was a large and complex test, and was a tough one to start with; this is very good work. I'm looking forward to some more test conversions this summer! Please verify that the updated code in the trunk looks correct in your sandbox after an 'svn update', and verify that you can build and run the new test from the trunk in your environment, and then please mark this issue closed.
        Hide
        Eranda Sooriyabandara added a comment -

        Hi Bryan,All are perfectly ran on my environment.
        Thanks Bryan, thanks for everything.
        I am closing this issue.

        Show
        Eranda Sooriyabandara added a comment - Hi Bryan,All are perfectly ran on my environment. Thanks Bryan, thanks for everything. I am closing this issue.

          People

          • Assignee:
            Eranda Sooriyabandara
            Reporter:
            Eranda Sooriyabandara
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 486h 5m
              486h 5m
              Remaining:
              Remaining Estimate - 486h 5m
              486h 5m
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development