Derby
  1. Derby
  2. DERBY-3458

dblook fails on TERRITORY_BASED databases

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.3.2.1
    • Fix Version/s: 10.3.3.0, 10.4.1.3
    • Component/s: Tools
    • Labels:
      None

      Description

      I've created small patches for myself by replacing all related queries in the 'tools' section with CASTs to CHARs and VARCHARs and would like to contribute these to the community in case anyone else can confirm this is a bug.

      A small test case to reproduce the problem is provided below, the version of Derby that provides the stacktrace is 10.3.2.1.

      Regards,

      Stephan van Loendersloot.

      Reproduction steps:

      ---------- 1: create_territory_db.sql ----------

      CONNECT 'jdbc:derby://localhost/dutch;user=dutch;password=dutch;create=true;territory=nl_NL;collation=TERRITORY_BASED';

      AUTOCOMMIT OFF;

      CREATE TABLE AIRLINES
      (
      AIRLINE CHAR(2) NOT NULL ,
      AIRLINE_FULL VARCHAR(24),
      BASIC_RATE DOUBLE PRECISION,
      DISTANCE_DISCOUNT DOUBLE PRECISION,
      BUSINESS_LEVEL_FACTOR DOUBLE PRECISION,
      FIRSTCLASS_LEVEL_FACTOR DOUBLE PRECISION,
      ECONOMY_SEATS INTEGER,
      BUSINESS_SEATS INTEGER,
      FIRSTCLASS_SEATS INTEGER
      );

      COMMIT;

      DISCONNECT;
      EXIT;

      ---------- 2: use dbloook ----------

      dblook -d "jdbc:derby://localhost/dutch;user=dutch;password=dutch" -o dutch.sql

      ---------- 3: stacktrace ----------

      java.sql.SQLSyntaxErrorException: Comparisons between 'CHAR (UCS_BASIC)' and 'CHAR (TERRITORY_BASED)' are not supported. Types must be comparable. String types must also have matching collation. If collation does not match, a possible solution is to cast operands to force them to the default collation (e.g. select tablename from sys.systables where CAST(tablename as VARCHAR(128)) = 'T1')
      at org.apache.derby.client.am.SQLExceptionFactory40.getSQLException(Unknown Source)
      at org.apache.derby.client.am.SqlException.getSQLException(Unknown Source)
      at org.apache.derby.client.am.Statement.executeQuery(Unknown Source)
      at org.apache.derby.tools.dblook.prepForDump(Unknown Source)
      at org.apache.derby.tools.dblook.go(Unknown Source)
      at org.apache.derby.tools.dblook.<init>(Unknown Source)
      at org.apache.derby.tools.dblook.main(Unknown Source)
      Caused by: org.apache.derby.client.am.SqlException: Comparisons between 'CHAR (UCS_BASIC)' and 'CHAR (TERRITORY_BASED)' are not supported. Types must be comparable. String types must also have matching collation. If collation does not match, a possible solution is to cast operands to force them to the default collation (e.g. select tablename from sys.systables where CAST(tablename as VARCHAR(128)) = 'T1')
      at org.apache.derby.client.am.Statement.completeSqlca(Unknown Source)
      at org.apache.derby.client.net.NetStatementReply.parsePrepareError(Unknown Source)
      at org.apache.derby.client.net.NetStatementReply.parsePRPSQLSTTreply(Unknown Source)
      at org.apache.derby.client.net.NetStatementReply.readPrepareDescribeOutput(Unknown Source)
      at org.apache.derby.client.net.StatementReply.readPrepareDescribeOutput(Unknown Source)
      at org.apache.derby.client.net.NetStatement.readPrepareDescribeOutput_(Unknown Source)
      at org.apache.derby.client.am.Statement.readPrepareDescribeOutput(Unknown Source)
      at org.apache.derby.client.am.Statement.flowExecute(Unknown Source)
      at org.apache.derby.client.am.Statement.executeQueryX(Unknown Source)
      ... 5 more
      – **--> DEBUG: Comparisons between 'CHAR (UCS_BASIC)' and 'CHAR (TERRITORY_BASED)' are not supported. Types must be comparable. String types must also have matching collation. If collation does not match, a possible solution is to cast operands to force them to the default collation (e.g. select tablename from sys.systables where CAST(tablename as VARCHAR(128)) = 'T1')

      1. DERBY-3458_patch_1.stat
        0.3 kB
        Stephan van Loendersloot
      2. DERBY-3458_patch_1.txt
        6 kB
        Stephan van Loendersloot
      3. DERBY-3458_patch_2.stat
        0.5 kB
        Stephan van Loendersloot
      4. DERBY-3458_patch_2.txt
        79 kB
        Stephan van Loendersloot
      5. DERBY-3458_patch_3_function_tests.txt
        112 kB
        Stephan van Loendersloot
      6. DERBY-3458_patch_3.stat
        2 kB
        Stephan van Loendersloot
      7. DERBY-3458_patch_3.txt
        1 kB
        Stephan van Loendersloot
      8. DERBY-3458_patch_4_function_tests.txt
        136 kB
        Stephan van Loendersloot
      9. DERBY-3458_patch_4.stat
        2 kB
        Stephan van Loendersloot
      10. DERBY-3458_patch_4.txt
        1 kB
        Stephan van Loendersloot

        Activity

        Hide
        Stephan van Loendersloot added a comment -

        The patch has changed queries in the required files as stated in my original report.

        I've added a functional test to the tools section which passes cleanly, but it isn't included in this patch, because I'm not sure how to proceed.

        I'd be glad if someone could inform me if I should create a new issue, or a subtask, or do something else to contribute that as well.

        Show
        Stephan van Loendersloot added a comment - The patch has changed queries in the required files as stated in my original report. I've added a functional test to the tools section which passes cleanly, but it isn't included in this patch, because I'm not sure how to proceed. I'd be glad if someone could inform me if I should create a new issue, or a subtask, or do something else to contribute that as well.
        Hide
        Mamta A. Satoor added a comment -

        Stephan, thanks for researhing AND fixing the issue. Great job. I looked through the code changes and they look good to me. You can go ahead and submit the functional test as part of this same jira entry.

        Show
        Mamta A. Satoor added a comment - Stephan, thanks for researhing AND fixing the issue. Great job. I looked through the code changes and they look good to me. You can go ahead and submit the functional test as part of this same jira entry.
        Hide
        Stephan van Loendersloot added a comment -

        The diff in patch_2 contains the function test for territory based databases. The original dblook_test.java has been slightly modified, to be able to extend it to the added dblook_test_territory.java.

        I've only tested the 'derbytools' suite (and it runs without errors), because my test environment is somehow not setup properly to run 'derbyAll', which hangs on network server tests. This doesn't seem to be due to this patch, it also happens when I test on released versions. I need to look into that some more, but I'd appreciate it if someone else can verify that 'derbyAll' works.

        Once I get my test environment right, I may add another function test which extends dblook_test_net.java, though for now, dblook works on our territory-based network servers (production).

        Show
        Stephan van Loendersloot added a comment - The diff in patch_2 contains the function test for territory based databases. The original dblook_test.java has been slightly modified, to be able to extend it to the added dblook_test_territory.java. I've only tested the 'derbytools' suite (and it runs without errors), because my test environment is somehow not setup properly to run 'derbyAll', which hangs on network server tests. This doesn't seem to be due to this patch, it also happens when I test on released versions. I need to look into that some more, but I'd appreciate it if someone else can verify that 'derbyAll' works. Once I get my test environment right, I may add another function test which extends dblook_test_net.java, though for now, dblook works on our territory-based network servers (production).
        Hide
        Mamta A. Satoor added a comment -

        Stephan, I did not think through completely when I sent my feedback on the patch(DERBY-3458_patch_1.txt) earlier today. The patch is CASTing the system table columns (all the system table character columns have UCS_BASIC collation associated with them) to current schema's collation and then comparing the resultant character string with a constant character string (constant character strings always take the collation of the schema in which the statement is getting compiled). This is really not we want. We want to ensure that when dealing with system table character columns, we use UCS_BASIC for collation. This may or may not happen with the supplied patch because we may or may not be system schema when the queries get compiled. In other words, the patch could cause Derby to use territory based collation for system columns rather than using UCS_BASIC collation because the current schema is user schema and not system schema. (I am not familiar enough with dblook to know what the current schema is when these queries are run).

        I think the correct solution is to make sure that we are in the system schema when these queries are compiled. That will ensure that the character string constants will take the collation of the system schema. This will also ensure that we do not run into collation mismatch errors.

        I apologize if you ended up wasting lot of time on this issue based on my earlier feedback.

        Show
        Mamta A. Satoor added a comment - Stephan, I did not think through completely when I sent my feedback on the patch( DERBY-3458 _patch_1.txt) earlier today. The patch is CASTing the system table columns (all the system table character columns have UCS_BASIC collation associated with them) to current schema's collation and then comparing the resultant character string with a constant character string (constant character strings always take the collation of the schema in which the statement is getting compiled). This is really not we want. We want to ensure that when dealing with system table character columns, we use UCS_BASIC for collation. This may or may not happen with the supplied patch because we may or may not be system schema when the queries get compiled. In other words, the patch could cause Derby to use territory based collation for system columns rather than using UCS_BASIC collation because the current schema is user schema and not system schema. (I am not familiar enough with dblook to know what the current schema is when these queries are run). I think the correct solution is to make sure that we are in the system schema when these queries are compiled. That will ensure that the character string constants will take the collation of the system schema. This will also ensure that we do not run into collation mismatch errors. I apologize if you ended up wasting lot of time on this issue based on my earlier feedback.
        Hide
        Stephan van Loendersloot added a comment -

        Hello Mamta,

        Thanks for taking time to review my proposed solution. I really don't think I've wasted time, since the functional test took more time than creating the patch, but it provided some useful insights on how this stuff currently works in Derby. Besides, I'll need the test anyway when trying to resolve this issue.

        Now, if I understand correctly, you're basically implying that all operations on system tables should be done with UCS_BASIC collation, which can be enforced by changing the current connection schema to SYS. I read about the UCS_BASIC part while lurking on the dev-list, but a proposed workaround seemed to use casting.

        Having investigated the internals of dblook, I found that only one connection to the database is made, which is passed on to all required (static) methods, with no other instances around. This would really make the patch a lot simpler.

        While poking around in a few other tests, but specifcally CollationTest.java, I came up with the idea to ensure that for 'dblook_test.java' the UCS_BASIC collation type is used right after the database has been created and for 'dblook_test_territory' the 'TERRITORY_BASED' collation type is used, just to be sure.

        This will affect the network test as well, but that doesn't have a territory-based test yet (as I still have to correct my environment), so I don't think it causes any problems.

        I'll make the changes and return shortly with a report and most likely a new patch.

        --Stephan.

        Show
        Stephan van Loendersloot added a comment - Hello Mamta, Thanks for taking time to review my proposed solution. I really don't think I've wasted time, since the functional test took more time than creating the patch, but it provided some useful insights on how this stuff currently works in Derby. Besides, I'll need the test anyway when trying to resolve this issue. Now, if I understand correctly, you're basically implying that all operations on system tables should be done with UCS_BASIC collation, which can be enforced by changing the current connection schema to SYS. I read about the UCS_BASIC part while lurking on the dev-list, but a proposed workaround seemed to use casting. Having investigated the internals of dblook, I found that only one connection to the database is made, which is passed on to all required (static) methods, with no other instances around. This would really make the patch a lot simpler. While poking around in a few other tests, but specifcally CollationTest.java, I came up with the idea to ensure that for 'dblook_test.java' the UCS_BASIC collation type is used right after the database has been created and for 'dblook_test_territory' the 'TERRITORY_BASED' collation type is used, just to be sure. This will affect the network test as well, but that doesn't have a territory-based test yet (as I still have to correct my environment), so I don't think it causes any problems. I'll make the changes and return shortly with a report and most likely a new patch. --Stephan.
        Hide
        Stephan van Loendersloot added a comment -

        Attached patch 3, as described in my earlier comment.

        Instead of returning shortly, I couldn't resist and figured out why my test-environment wasn't setup properly, so I could add function tests to the suite 'derbynetmats' as well.

        The reason why I couldn't get it right the first time, is because the documentation at testing/README.htm refers to an incorrect download location for 'db2jcc.jar' and 'db2jcc_license_c.jar'. At that location, DB2 drivers for Linux can be downloaded, but they fail the Derby tests.

        It might be wise to update the readme and point to the right location, but maybe that's something for a new issue.

        Anyway.. now both 'derbytools' and 'derbynetmats' pass the tests... I will run 'derbyall' later this night and review the outcome in the morning.

        In the meantime I'd be happy to hear someone else confirming that this patch is working.

        Thanks,

        --Stephan.

        Show
        Stephan van Loendersloot added a comment - Attached patch 3, as described in my earlier comment. Instead of returning shortly, I couldn't resist and figured out why my test-environment wasn't setup properly, so I could add function tests to the suite 'derbynetmats' as well. The reason why I couldn't get it right the first time, is because the documentation at testing/README.htm refers to an incorrect download location for 'db2jcc.jar' and 'db2jcc_license_c.jar'. At that location, DB2 drivers for Linux can be downloaded, but they fail the Derby tests. It might be wise to update the readme and point to the right location, but maybe that's something for a new issue. Anyway.. now both 'derbytools' and 'derbynetmats' pass the tests... I will run 'derbyall' later this night and review the outcome in the morning. In the meantime I'd be happy to hear someone else confirming that this patch is working. Thanks, --Stephan.
        Hide
        Stephan van Loendersloot added a comment -

        Marking patch as unavailable. The test functionality for 'derbyall' doesn't complete. It looks like I need to rename the output of dblook after each test to prevent successive tests from failing. Will be back with a new patch when 'derbyall' completes successfully.

        Show
        Stephan van Loendersloot added a comment - Marking patch as unavailable. The test functionality for 'derbyall' doesn't complete. It looks like I need to rename the output of dblook after each test to prevent successive tests from failing. Will be back with a new patch when 'derbyall' completes successfully.
        Hide
        Mamta A. Satoor added a comment -

        Stephan, I looked at DERBY-3458_patch_3.txt and it looks good. It's good that we get that only one connection is made which makes the fix clean. Once you have the functional test, I will be happy to apply your patch and run the tests on my machine.

        Show
        Mamta A. Satoor added a comment - Stephan, I looked at DERBY-3458 _patch_3.txt and it looks good. It's good that we get that only one connection is made which makes the fix clean. Once you have the functional test, I will be happy to apply your patch and run the tests on my machine.
        Hide
        Stephan van Loendersloot added a comment -

        Hello again, and my apologies for the delay, my daily job required all of my attention.

        I attached a cumulative patch v4, which replaces all previous patches. All functional tests (derbyall) run without errors. Important changes are only in the tests, which now rename the output of dblook after completion, instead of at the start. In addition, the master test for DerbyNetClient needed a .out file.

        Since multiple tests may run, it was insufficient to just rename dblook.log to f.e. dblook_test_net.log. The central method renameDbLookLog() in dblook_test.java now renames to dblook_test_net0.log, dblook_test_net1.log and so on.

        Please note that I haven't run tests for optional suites like mobile environments, though modifications have been made to incorporate them.

        Regards,

        Stephan.

        Show
        Stephan van Loendersloot added a comment - Hello again, and my apologies for the delay, my daily job required all of my attention. I attached a cumulative patch v4, which replaces all previous patches. All functional tests (derbyall) run without errors. Important changes are only in the tests, which now rename the output of dblook after completion, instead of at the start. In addition, the master test for DerbyNetClient needed a .out file. Since multiple tests may run, it was insufficient to just rename dblook.log to f.e. dblook_test_net.log. The central method renameDbLookLog() in dblook_test.java now renames to dblook_test_net0.log, dblook_test_net1.log and so on. Please note that I haven't run tests for optional suites like mobile environments, though modifications have been made to incorporate them. Regards, Stephan.
        Hide
        Mamta A. Satoor added a comment -

        Just an FYI that I will look at the patch tomorrow and if everything runs fine, I will go ahead and check it in.

        Thanks, Stephan, for your work on this jira entry. Also, just wanted to mention that you might consider submitting an ICLA with ASF if you plan on contributing more to Derby.

        These pages on the Derby wiki has information for getting involved in the Derby project.
        http://wiki.apache.org/db-derby/DerbyDev
        http://wiki.apache.org/db-derby/DerbyContributorChecklist

        Show
        Mamta A. Satoor added a comment - Just an FYI that I will look at the patch tomorrow and if everything runs fine, I will go ahead and check it in. Thanks, Stephan, for your work on this jira entry. Also, just wanted to mention that you might consider submitting an ICLA with ASF if you plan on contributing more to Derby. These pages on the Derby wiki has information for getting involved in the Derby project. http://wiki.apache.org/db-derby/DerbyDev http://wiki.apache.org/db-derby/DerbyContributorChecklist
        Hide
        Mamta A. Satoor added a comment -

        Committed changes submitted by Stephan using revision 634037 into trunk. I will work on backporting it to 10.3

        Show
        Mamta A. Satoor added a comment - Committed changes submitted by Stephan using revision 634037 into trunk. I will work on backporting it to 10.3
        Hide
        Dyre Tjeldvoll added a comment -

        Patch has been committed.

        Show
        Dyre Tjeldvoll added a comment - Patch has been committed.
        Hide
        Stephan van Loendersloot added a comment -

        Hi Mamta (and others),

        Thanks for reviewing and committing the patch, everything works fine now, so I'll close this bug.

        I will be looking into submitting an ICLA with the ASF, a couple of potential features and bugs caught my interest.

        I'd first like to try to make myself more familiar with the Derby codebase, if and when time permits.

        Thanks again.

        -Stephan.

        Show
        Stephan van Loendersloot added a comment - Hi Mamta (and others), Thanks for reviewing and committing the patch, everything works fine now, so I'll close this bug. I will be looking into submitting an ICLA with the ASF, a couple of potential features and bugs caught my interest. I'd first like to try to make myself more familiar with the Derby codebase, if and when time permits. Thanks again. -Stephan.
        Hide
        Mamta A. Satoor added a comment -

        Migrated the changes into 10.3 codeline with revision 635588.

        Show
        Mamta A. Satoor added a comment - Migrated the changes into 10.3 codeline with revision 635588.
        Hide
        Kathey Marsden added a comment -

        Adding 10.3.2.2 to fix version as this fix was ported to the 10.3 branch.

        Show
        Kathey Marsden added a comment - Adding 10.3.2.2 to fix version as this fix was ported to the 10.3 branch.

          People

          • Assignee:
            Stephan van Loendersloot
            Reporter:
            Stephan van Loendersloot
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development