Derby
  1. Derby
  2. DERBY-4430

Make ij's SHOW and DESCRIBE commands more db agnostic

    Details

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

      Description

      ij's SHOW and DESCRIBE commands use DatabaseMetaData in order to be portable to other databases than Derby, and in many cases they work fine with other databases. However, the variants of the commands that take a table name or a schema name assume that unquoted identifiers are converted to and stored in upper case internally. This is not true for all databases, and since these commands don't accept quoted identifiers, there's currently no way to access tables/schemas that are not all upper case.

      One possible fix is to make the identifier() method in ij.jj use the DatabaseMetaData methods storesLowerCaseIdentifiers(), storesMixedCaseIdentifiers() and storesUpperCaseIdentifiers() to decide whether it should convert the identifier to lower case, keep it unchanged, or convert it to upper case. Currently, that method always converts the identifiers to upper case.

      1. DERBY-4430.sql
        0.3 kB
        Sylvain Leroux
      2. DERBY-4430.patch
        5 kB
        Sylvain Leroux
      3. DERBY-4430.ij.out
        2 kB
        Sylvain Leroux

        Activity

        Hide
        Knut Anders Hatlen added a comment -

        Example using ij to access a PostgreSQL database:

        ij version 10.6
        ij> connect 'jdbc:postgresql://localhost/test';
        ij> create table my_table(x int);
        0 rows inserted/updated/deleted
        ij> show tables;
        table_schem |table_name |remarks
        ------------------------------------------------------------------------
        information_schema |sql_features |NULL
        information_schema |sql_implementation_info |NULL
        .
        .
        .
        pg_catalog |pg_type |NULL
        public |my_table |NULL

        48 rows selected

        The newly created table (my_table) shows up in the output from "SHOW TABLES", but it's stored in lower case. Showing only the tables in the public schema is not possible, since ij converts the identifier to upper case:

        ij> show tables in public;
        table_schem |table_name |remarks
        ------------------------------------------------------------------------

        0 rows selected

        And ij only supports unquoted identifiers:

        ij> show tables in "public";
        ERROR 42601: ERROR: syntax error at or near "in"

        Show
        Knut Anders Hatlen added a comment - Example using ij to access a PostgreSQL database: ij version 10.6 ij> connect 'jdbc:postgresql://localhost/test'; ij> create table my_table(x int); 0 rows inserted/updated/deleted ij> show tables; table_schem |table_name |remarks ------------------------------------------------------------------------ information_schema |sql_features |NULL information_schema |sql_implementation_info |NULL . . . pg_catalog |pg_type |NULL public |my_table |NULL 48 rows selected The newly created table (my_table) shows up in the output from "SHOW TABLES", but it's stored in lower case. Showing only the tables in the public schema is not possible, since ij converts the identifier to upper case: ij> show tables in public; table_schem |table_name |remarks ------------------------------------------------------------------------ 0 rows selected And ij only supports unquoted identifiers: ij> show tables in "public"; ERROR 42601: ERROR: syntax error at or near "in"
        Hide
        Sylvain Leroux added a comment -

        Hi,

        Here is a first fix for this issue.
        As Knut suggested, it uses 'stores

        {Upper,Lower}

        CaseIdentifiers'. However, I'm not sure it's very efficient to call getMetaData each time an identifier is encountered.

        The other two attachments are:

        • DERBY-4430.sql: the SQL file I used to manually test that patch against Postgre8.3
        • DERBY-4430.ij.out: the output of ij when running this file

        I've added a test in ij7 to ensure that lower case identifiers are still converted to upper case when we are running ij against Derby.
        This test passed - but only after I removed two white lines at the top of ij7.sql ???

        One thing I was wondering: why using the ENGLISH locale when converting an identifier to upper/lower case?

        Show
        Sylvain Leroux added a comment - Hi, Here is a first fix for this issue. As Knut suggested, it uses 'stores {Upper,Lower} CaseIdentifiers'. However, I'm not sure it's very efficient to call getMetaData each time an identifier is encountered. The other two attachments are: DERBY-4430 .sql: the SQL file I used to manually test that patch against Postgre8.3 DERBY-4430 .ij.out: the output of ij when running this file I've added a test in ij7 to ensure that lower case identifiers are still converted to upper case when we are running ij against Derby. This test passed - but only after I removed two white lines at the top of ij7.sql ??? One thing I was wondering: why using the ENGLISH locale when converting an identifier to upper/lower case?
        Hide
        Knut Anders Hatlen added a comment -

        Thanks for the patch, Sylvain! I had a quick look at it and it looks good. I'll give it a try.

        To your questions:

        1) I wouldn't worry too much about the calls to getMetaData(). Both the client driver and the embedded driver cache the DatabaseMetaData object in the Connection object, and DatabaseMetaData caches the values returned by stores*CaseIdentifiers, so I think there would be little to gain by caching them ourselves in ij.

        2) The ij7.sql test is run by the ToolScripts JUnit test. I see the same failure as you if I run it with the old style runner, o.a...functionTests.harness.RunTest, but not if I run it using the JUnit runner:

        java junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.tools.ToolScripts

        3) The English locale is used to prevent the JVM's default locale from influencing how the identifiers are handled. For instance, in Turkish locale, toUpper("insert") returns "İNSERT" (note the upper-case i with a dot), and toLower("INSERT") returns "ınsert" (dot-less lower-case i).

        Show
        Knut Anders Hatlen added a comment - Thanks for the patch, Sylvain! I had a quick look at it and it looks good. I'll give it a try. To your questions: 1) I wouldn't worry too much about the calls to getMetaData(). Both the client driver and the embedded driver cache the DatabaseMetaData object in the Connection object, and DatabaseMetaData caches the values returned by stores*CaseIdentifiers, so I think there would be little to gain by caching them ourselves in ij. 2) The ij7.sql test is run by the ToolScripts JUnit test. I see the same failure as you if I run it with the old style runner, o.a...functionTests.harness.RunTest, but not if I run it using the JUnit runner: java junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.tools.ToolScripts 3) The English locale is used to prevent the JVM's default locale from influencing how the identifiers are handled. For instance, in Turkish locale, toUpper("insert") returns "İNSERT" (note the upper-case i with a dot), and toLower("INSERT") returns "ınsert" (dot-less lower-case i).
        Hide
        Knut Anders Hatlen added a comment -

        I've tested the patch with MySQL (where storesMixedCaseIdentifier() returns true) and PostgreSQL (where storesLowerCaseIdentifier() returns true). The SHOW and DESCRIBE commands now work fine on those databases too.

        Show
        Knut Anders Hatlen added a comment - I've tested the patch with MySQL (where storesMixedCaseIdentifier() returns true) and PostgreSQL (where storesLowerCaseIdentifier() returns true). The SHOW and DESCRIBE commands now work fine on those databases too.
        Hide
        Knut Anders Hatlen added a comment -

        I ran all the regression tests, and found that there's one problem that makes many of the derbyall tests fail. The problem is that the identifier() method now requires that there is a connection, whereas it is used by some ij commands that don't require that there is currently a connection to the database. One example is the CONNECT command, which now fails if the connection is named:

        ij> connect 'jdbc:derby:test;create=true' as c1;
        IJ ERROR: Unable to establish connection

        I think we need to make sure that the new identifier() method is only used by the SHOW and DESCRIBE commands that reference objects in the database.

        Show
        Knut Anders Hatlen added a comment - I ran all the regression tests, and found that there's one problem that makes many of the derbyall tests fail. The problem is that the identifier() method now requires that there is a connection, whereas it is used by some ij commands that don't require that there is currently a connection to the database. One example is the CONNECT command, which now fails if the connection is named: ij> connect 'jdbc:derby:test;create=true' as c1; IJ ERROR: Unable to establish connection I think we need to make sure that the new identifier() method is only used by the SHOW and DESCRIBE commands that reference objects in the database.
        Hide
        Sylvain Leroux added a comment -

        Thanks for all these explanations.

        Here is a second version of the patch. Now, I let the identifier() method untouched. But I have introduced an other method caIdentifier() (for case-aware identifier). Only the latter uses database meta data to check for the case policy of the DB.

        I've changed the rules for DESCRIBE and SHOW to use that new method instead of the standard one.

        Concerning the tests, it works against Postgre8.3. I join the corresponding SQL and IJ output.

        I ran ToolScripts (with junit) without any error.
        I ran derbyall (harness-style) with only an error with "bootLock" . Here is the relevant part of the report:

                        • Diff file storeall/storemore/bootLock.diff
            • Start: bootLock jdk1.6.0_12 storeall:storemore 2009-11-14 12:05:11 ***
              2,4d1
              < expected exception
              < SQLSTATE(XJ040):
              < SQLSTATE(XSDB6):
              Test Failed.
            • End: bootLock jdk1.6.0_12 storeall:storemore 2009-11-14 12:05:44 ***
                        • Diff file derbyall/storeall/storemore/bootLock.diff
            • Start: bootLock jdk1.6.0_12 storeall:storemore 2009-11-14 12:49:45 ***
              2,4d1
              < expected exception
              < SQLSTATE(XJ040):
              < SQLSTATE(XSDB6):
              Test Failed.
        Show
        Sylvain Leroux added a comment - Thanks for all these explanations. Here is a second version of the patch. Now, I let the identifier() method untouched. But I have introduced an other method caIdentifier() (for case-aware identifier). Only the latter uses database meta data to check for the case policy of the DB. I've changed the rules for DESCRIBE and SHOW to use that new method instead of the standard one. Concerning the tests, it works against Postgre8.3. I join the corresponding SQL and IJ output. I ran ToolScripts (with junit) without any error. I ran derbyall (harness-style) with only an error with "bootLock" . Here is the relevant part of the report: Diff file storeall/storemore/bootLock.diff Start: bootLock jdk1.6.0_12 storeall:storemore 2009-11-14 12:05:11 *** 2,4d1 < expected exception < SQLSTATE(XJ040): < SQLSTATE(XSDB6): Test Failed. End: bootLock jdk1.6.0_12 storeall:storemore 2009-11-14 12:05:44 *** Diff file derbyall/storeall/storemore/bootLock.diff Start: bootLock jdk1.6.0_12 storeall:storemore 2009-11-14 12:49:45 *** 2,4d1 < expected exception < SQLSTATE(XJ040): < SQLSTATE(XSDB6): Test Failed.
        Hide
        Knut Anders Hatlen added a comment -

        Thanks, Sylvain! The patch looks good to me. I'll run the tests and commit it if I don't see any failures. I've made one change to the patch, re-introducing the comment that was removed from the original identifier() method.

        The failure you're seeing in bootLock is probably not related to the patch. I found a bug report, DERBY-4179, which described the same failure. It was closed because it only happened once, but I've reopened it and pointed it to this issue.

        Show
        Knut Anders Hatlen added a comment - Thanks, Sylvain! The patch looks good to me. I'll run the tests and commit it if I don't see any failures. I've made one change to the patch, re-introducing the comment that was removed from the original identifier() method. The failure you're seeing in bootLock is probably not related to the patch. I found a bug report, DERBY-4179 , which described the same failure. It was closed because it only happened once, but I've reopened it and pointed it to this issue.
        Hide
        Knut Anders Hatlen added a comment -

        All the tests ran cleanly in my environment. Committed revision 836277.

        Thanks for working on this, Sylvain!

        Show
        Knut Anders Hatlen added a comment - All the tests ran cleanly in my environment. Committed revision 836277. Thanks for working on this, Sylvain!

          People

          • Assignee:
            Sylvain Leroux
            Reporter:
            Knut Anders Hatlen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development