|
I kind of assumed 'schemaName' was an identifier. The description for identifier says this (http://db.apache.org/derby/docs/dev/tools/tools-single.html#rtoolsijcomref40155):
"Identifier Syntax Identifier Description Some ij commands require identifiers. These ij identifiers are case-insensitive. They must begin with a letter in the range A-Z, and can consist of any number of letters in the range A-Z, digits in the range 0-9, and underscore (_) characters. These identifiers exist within the scope of ij only and are distinct from any identifiers used in SQL commands, except in the case of the Get Cursor command. The Get Cursor command specifies a cursor name to use in creating a result set. ij does not recognize or permit delimited identifiers in ij commands. They can be used in SQL commands. Example These are valid ij identifiers: foo1 exampleIdentifier12345 another_one" Maybe I'm wrong, but it's a bit hard to understand from the documentation. Some identifiers are linked to the page I mentioned, some are not. and others again might not be identifiers at all! If th latter is the case, then show indexes also has bugs in the handling of delimited/quoted table and schema names. The documentation for the SHOW and DESCRIBE commands does not use the term "Identifier" at all, so I did not think about looking up that in the manual. But if table-Name, view-Name and schemaName are considered identifiers, and double quotes are considered delimiters, then I think these issues cannot be considered as bugs per se. But the documentation should clarify this, and I would be happy to see support for this getting improved.
As it is today, the SHOW and DESCRIBE commands cannot be used for case-sensitive names. Some tools (such as the NetBeans 5.0/5.5 Derby/Java DB module) put quotes around such names by default, making this a potential issue for many users. The reason why this works for the embedded driver but not the client one is that the client calls org/apache/derby/client/am/DatabaseMetaData#getIndexInfo which throws SQLException if table == null. The embedded client calls the org/apache/derby/impl/jdbc/EmbedDatabaseMetaData#getIndexInfo which does not perform this check.
According to the JDBC API, the getIndexInfo methods should not allow null values for the table name. Hence, as part of issue 1484, EmbedDatabaseMetaData#getIndexInfo will be changed to throw an SQLException as well. However, the ij reference says that (http://db.apache.org/derby/docs/dev/tools/rtoolsijcomrefshow.html): SHOW { CONNECTIONS | INDEXES [ IN schemaName | FROM table-Name ] | PROCEDURES [ IN schemaName ] | SCHEMAS | SYNONYMS [ IN schemaName ] | TABLES [ IN schemaName ] | VIEWS [ IN schemaName ] | } (...) SHOW INDEXES displays all the indexes in the database. If IN schemaName is specified, then only the indexes in the specified schema are displayed. This is the same behavior currently observed by the embedded client, but not after issue 1484 is resolved. How should this best be solved? Two possible solutions are: 1) Change the ij reference so that "show indexes in app;" is not allowed. 2) When "show indexes in app" is performed, first get all tables in schema app, and then get all indexes for the tables one at a time. This would probably require a means to merge multiple resultsets into one ijResult. Is this possible with any existing Class? By fixing
To remedy this, the "show index" functionality has been rewritten to something like: ---- code --- ij.jj#showIndexes (schema, table){ if (table != null) return new ijResultSetResult(dbmd.getIndexInfo(schema, table)) else dbmd.getTables(schema) for each tablename in schema resultSetList.add(dbmd.getIndexInfo(schema, table)) return new ijMultipleResultSetResult(resultSetList) ---- /code --- The posted patch adds a new subclass of ijResultImpl (currently called ijMultipleResultSetResult) that effectively is a List of ResultSet. JDBCDisplayUtil is modified to handle this by showing the banner first, and then show all the rows in all resultsets in the ijMultipleResultSetResult object. I will apply the same fix to all ij functionality that uses methods in DatabaseMetaData where table = null is no longer allowed. See I forgot; this also solves issue 2222: both client and embedded can now perform "show indexes in SCHEMANAME" without getting
ERROR XJ103: Table name can not be null as described in the ij reference Hi Jørgen,
I think you have found a good solution. Some comments to the attached code: 1) If you do "svn add" on ijMultipleResultSetResult.java, it will be included in the output from "svn diff" and you don't need to attach the file separately. 2) indent_DisplayMultipleResults() seems to be very similar to indent_DisplayResults(). Is it possible to make indent_DisplayResults() more general (for instance, let it take an array/list of result sets) and use that method both in DisplayResults() and DisplayMultipleResults()? 3) Derby code normally uses 4 blanks for indentation. 4) Perhaps you could add a test case for "show indexes in schemaname" so it is tested by the regression tests. Some other usages of "show indexes" are tested in tools/ij7.sql. Thanks for the comments, Knut. I have addressed them in patch
Since I am new to Derby, I am not sure if all the following test information is required, but to much info is probably better than to little: Patch (2) initially failed derbyall (store/bootLock.java) and allsuites (synonym.sql in LangScripts); in the synonym test "drop synonym mySyn;" (line 237 in synonym.sql), the views were listed in the reverse order compared to the master file. Both failures disappeared after ant clobber/svn update. The full tests have not been rerun after this. > Knut Anders Hatlen commented on > ------------------------------------------- > > Hi Jørgen, > I think you have found a good solution. Some comments to the attached code: > 1) If you do "svn add" on ijMultipleResultSetResult.java, it will be included in the output from "svn diff" and you don't need to attach the file separately. Done. Thanks for the tip. > 2) indent_DisplayMultipleResults() seems to be very similar to indent_DisplayResults(). Is it possible to make indent_DisplayResults() more general (for instance, let it take an array/list of result sets) and use that method both in DisplayResults() and DisplayMultipleResults()? A good point. I more or less cut'n pasted that method in the first diff file. I have now rewritten these to one single method. > 3) Derby code normally uses 4 blanks for indentation. Fixed > 4) Perhaps you could add a test case for "show indexes in schemaname" so it is tested by the regression tests. Some other usages of "show indexes" are tested in tools/ij7.sql. Added one test to this file. I'm considering committing the v2 patch, but have a few comments. The tests run cleanly, so the comments are more like nits.
1) Some new code seem to be mixing tabs and spaces for indentation. The current guidelines can be found here: http://www.nabble.com/-VOTE---Approve-coding-conventions-for-the-Derby-project-p5771191.html. I always use spaces if I change more than a few consecutive lines, even if the rest of the file has tabs. 2) Lines longer than 80 characters. 3) The name of the methods 'getDisplayColumns' and 'getColumnWidths' are very generic, where as their comments indicate they are special methods that are only used for displaying index information. Can the names or the comments be changed (as appropriate) to be more in agreement? 4) The comment " //TODO: may want to change tabletype" in 'ij.jj' doesn't make much sense to me. Do I need to study the code to figure out what it means, or can it be made clearer/removed? 5) For empty method bodies, it might be nice with a one-liner explaining why it is empty. For instance, I assume the method 'clearSQLWarnings' in 'ijMultipleResultSetResult.java' is a no-op because it can't get any warnings. 6) The class 'ijMultipleResultSetResult' does not follow common naming guidelines for Java classes, since the start of the name is in lower case. I observe that this is the case for quite a lot of the files in the java/tools directories. 7) I wasn't able to quickly determine if the modified test (ij7.sql) is run with the client driver. Do you know? I did search a derbyall_pass.txt file (on a run with no errors), but only found the test listed once. I'll wait a little for feedback on my comments before I commit. thanks, Thanks for reviewing the patch, Kristian.
I have made a new patch that will supersed (2), and will attach it as soon as the tests have terminated. The patch addresses your comments: 1-4) Fixed 5) Ohh... forgot these! Thanks. The methods are modified in the new patch. 6) I know, but I considered it better to continue the naming convention used for ij*.java files. Not changed. 7) You are right, and as far as i can tell the other tests in ij7.sql are performed only in embedded mode as well. I will start a subtask to address this. Kristian Waagan (JIRA) wrote: > I'm considering committing the v2 patch, but have a few comments. The tests run cleanly, so the comments are more like nits. > > 1) Some new code seem to be mixing tabs and spaces for indentation. The current guidelines can be found here: http://www.nabble.com/-VOTE---Approve-coding-conventions-for-the-Derby-project-p5771191.html. I always use spaces if I change more than a few consecutive lines, even if the rest of the file has tabs. > > 2) Lines longer than 80 characters. > > 3) The name of the methods 'getDisplayColumns' and 'getColumnWidths' are very generic, where as their comments indicate they are special methods that are only used for displaying index information. Can the names or the comments be changed (as appropriate) to be more in agreement? > > 4) The comment " //TODO: may want to change tabletype" in 'ij.jj' doesn't make much sense to me. Do I need to study the code to figure out what it means, or can it be made clearer/removed? > > 5) For empty method bodies, it might be nice with a one-liner explaining why it is empty. For instance, I assume the method 'clearSQLWarnings' in 'ijMultipleResultSetResult.java' is a no-op because it can't get any warnings. > > 6) The class 'ijMultipleResultSetResult' does not follow common naming guidelines for Java classes, since the start of the name is in lower case. I observe that this is the case for quite a lot of the files in the java/tools directories. > > 7) I wasn't able to quickly determine if the modified test (ij7.sql) is run with the client driver. Do you know? I did search a derbyall_pass.txt file (on a run with no errors), but only found the test listed once. All tests, except bootLock, run flawlessly. Patch (3) is now ready for review.
Committed '
Merged to 10.2 with revision 536107. Ran derbyall/suitesall (1.4, 1.5 and 1.6) for trunk, run for 10.2 is in progress now. Will report back and take corrective action if it shows problems. Committed 'derby-2222-4a-javadoc_whitespace.diff' with a few JavaDoc and formatting fixes (space/tabs, long lines) to trunk and 10.2 (revisions 536111 and 536112).
Thanks for committing this patch, Kristian.
Verified fix in client driver.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Major
'show indexes in SCHEMANAME' does not work with the client driver
-----------------------------------------------------------------
> Using delimited table and/or schema names does not work, but I think this is according to documentation.
What documentation are you referring to here? I think this is related to
DERBY-2019(the DESCRIBE command does not handle delimited/quoted names), but I was not able to find anything in the manuals saying this should not work.