Derby
  1. Derby
  2. DERBY-4115

Provide a way to drop statistics information

    Details

    • Issue & fix info:
      High Value Fix

      Description

      Now that DERBY-269 has been resolved, users can update statistics, but once they do, they are committed to using and maintaining the statistics, even if it doesn't improve performance or they have difficulty maintaining the statistics on a regular basis. It would be good to have a way to drop statistics information so that users could revert to the prior behavior if needed.

      1. DERBY4115_patch1_diff.txt
        4 kB
        Mamta A. Satoor
      2. DERBY4115_patch2_diff.txt
        7 kB
        Mamta A. Satoor
      3. DERBY4115_patch3_diff.txt
        11 kB
        Mamta A. Satoor
      4. DERBY4115_patch4_diff.txt
        18 kB
        Mamta A. Satoor
      5. DERBY4115_patch5_diff.txt
        34 kB
        Mamta A. Satoor
      6. DERBY4115_patch6_diff.txt
        52 kB
        Mamta A. Satoor
      7. derby-4115-7a-move_test.diff
        13 kB
        Kristian Waagan

        Issue Links

        There are no Sub-Tasks for this issue.

          Activity

          Hide
          Kristian Waagan added a comment -

          This seems like a simple system procedure to implement, and could be useful now that automatic updates of index cardinality statistics is enabled by default. We just saw a user report where the statistics were messed up due to a bug in Derby, and this caused the istat daemon to go crazy. A bug in the daemon itself would also be easier to work around if it is possible to discard existing statistics after the daemon has been disabled.

          Has this feature never been implemented due to lack of demand, or are there potential issues that haven't been noted in this JIRA?

          Show
          Kristian Waagan added a comment - This seems like a simple system procedure to implement, and could be useful now that automatic updates of index cardinality statistics is enabled by default. We just saw a user report where the statistics were messed up due to a bug in Derby, and this caused the istat daemon to go crazy. A bug in the daemon itself would also be easier to work around if it is possible to discard existing statistics after the daemon has been disabled. Has this feature never been implemented due to lack of demand, or are there potential issues that haven't been noted in this JIRA?
          Hide
          Mike Matrigali added a comment -

          Of course can't say without seeing an implementation, but I can't think of potential issues for not implementing this. And give
          DERBY-5681 it seems even more useful. One problem is I don't think we can backport this feature, as adding a new system procedure
          would create compatibility/upgrade issues. So would not help 10.8 users but would help in the future for 10.9 and above.

          And it would be great if the implementation handled the unexpected rows caused by DERBY-5681, since even after this bug is fixed
          we may always in the future still see these rows as people upgrade old dbs to new derby versions.

          Show
          Mike Matrigali added a comment - Of course can't say without seeing an implementation, but I can't think of potential issues for not implementing this. And give DERBY-5681 it seems even more useful. One problem is I don't think we can backport this feature, as adding a new system procedure would create compatibility/upgrade issues. So would not help 10.8 users but would help in the future for 10.9 and above. And it would be great if the implementation handled the unexpected rows caused by DERBY-5681 , since even after this bug is fixed we may always in the future still see these rows as people upgrade old dbs to new derby versions.
          Hide
          Mike Matrigali added a comment -

          DERBY-5681 can leave bad rows in statistics catalog, would be good to have a way to manually get rid of this bad row. I think it would be hard to implement going after just the bad row as I think it references an object that does not exist anymore. So seems sufficient to provide a way to drop all statistics, and have the implementation handle this unexpected row also.

          Show
          Mike Matrigali added a comment - DERBY-5681 can leave bad rows in statistics catalog, would be good to have a way to manually get rid of this bad row. I think it would be hard to implement going after just the bad row as I think it references an object that does not exist anymore. So seems sufficient to provide a way to drop all statistics, and have the implementation handle this unexpected row also.
          Hide
          Kathey Marsden added a comment -

          >Has this feature never been implemented due to lack of demand, or are there potential issues that haven't been noted in this JIRA?

          I filed it in response to a specific user case where it would have been helpful. Now with automatic index stat it is more needed. I think it hasn't been picked up before now because folks just haven't had the bandwidth and it is not a bug so doesn't get reviewed often with triage. I think it would be a great addition and am marking it High Value Fix in case anyone finds that inspiring #

          Show
          Kathey Marsden added a comment - >Has this feature never been implemented due to lack of demand, or are there potential issues that haven't been noted in this JIRA? I filed it in response to a specific user case where it would have been helpful. Now with automatic index stat it is more needed. I think it hasn't been picked up before now because folks just haven't had the bandwidth and it is not a bug so doesn't get reviewed often with triage. I think it would be a great addition and am marking it High Value Fix in case anyone finds that inspiring #
          Hide
          Mamta A. Satoor added a comment -

          Are we talking about dropping statistics for a specific table or all the tables in a given schema? I think either could be implemented but wanted to double check the scope of the procedure.

          Show
          Mamta A. Satoor added a comment - Are we talking about dropping statistics for a specific table or all the tables in a given schema? I think either could be implemented but wanted to double check the scope of the procedure.
          Hide
          Mike Matrigali added a comment -

          I suggest an incremental approach, but add enough arguments for future expansion. I would think a good first step would be a call that dropped all the statistics for a single table. So include schema, table and indexname arguments, but for first step only support null for
          indexname which would mean drop all stats for a table.

          Next step might be to support dropping just those stats on a particular index.

          Show
          Mike Matrigali added a comment - I suggest an incremental approach, but add enough arguments for future expansion. I would think a good first step would be a call that dropped all the statistics for a single table. So include schema, table and indexname arguments, but for first step only support null for indexname which would mean drop all stats for a table. Next step might be to support dropping just those stats on a particular index.
          Hide
          Mamta A. Satoor added a comment -

          This is first incremental patch(not ready for commit). It just has the very basic prototype for a stored procedure which will eventually drop the statistics for a given table or it's index. The procedure at this point just has a println priting the parameters that were passed to it.

          I have added one call to this procedure to make sure that it gets invoked without any error.

          Next, I plan to write upgrade test to make sure that this method will not be available on pre-10.9 dbs, soft=upgraded dbs. It should only be available on hard upgraded dbs.

          Show
          Mamta A. Satoor added a comment - This is first incremental patch(not ready for commit). It just has the very basic prototype for a stored procedure which will eventually drop the statistics for a given table or it's index. The procedure at this point just has a println priting the parameters that were passed to it. I have added one call to this procedure to make sure that it gets invoked without any error. Next, I plan to write upgrade test to make sure that this method will not be available on pre-10.9 dbs, soft=upgraded dbs. It should only be available on hard upgraded dbs.
          Hide
          Kristian Waagan added a comment -

          > Next, I plan to write upgrade test to make sure that this method will not be available on pre-10.9 dbs, soft=upgraded dbs. It should only be available on hard upgraded dbs.

          It's been a while since I looked into upgrade and stored procedures. Does the restriction above come from the fact that we can't support the alternative behavior? That would be to allow the procedure to be called on a soft upgraded database to remove orphaned statistics, and then continue using the older version afterwards. This way the problem can be fixed without moving off the current Derby version.
          Even if the above is technically possible, I'm fine with forbidding it if it conflicts with our current policy. I was just a bit curious

          Show
          Kristian Waagan added a comment - > Next, I plan to write upgrade test to make sure that this method will not be available on pre-10.9 dbs, soft=upgraded dbs. It should only be available on hard upgraded dbs. It's been a while since I looked into upgrade and stored procedures. Does the restriction above come from the fact that we can't support the alternative behavior? That would be to allow the procedure to be called on a soft upgraded database to remove orphaned statistics, and then continue using the older version afterwards. This way the problem can be fixed without moving off the current Derby version. Even if the above is technically possible, I'm fine with forbidding it if it conflicts with our current policy. I was just a bit curious
          Hide
          Mamta A. Satoor added a comment -

          I forgot to mention in my earlier patch that I have given the new routine(SYSCS_DROP_STATISTICS) public access (same as existing update statistics procedure). This happens in DataDictionaryImpl, where SYSCS_DROP_STATISTICS is added to the list of public access procedures in sysUtilProceduresWithPublicAccess. Just wanted to bring this up since it is not very obvious. Thanks

          Also, please let me know if we should use a different name for this procedure. I am using SYSCS_DROP_STATISTICS in the attached patch.

          Show
          Mamta A. Satoor added a comment - I forgot to mention in my earlier patch that I have given the new routine(SYSCS_DROP_STATISTICS) public access (same as existing update statistics procedure). This happens in DataDictionaryImpl, where SYSCS_DROP_STATISTICS is added to the list of public access procedures in sysUtilProceduresWithPublicAccess. Just wanted to bring this up since it is not very obvious. Thanks Also, please let me know if we should use a different name for this procedure. I am using SYSCS_DROP_STATISTICS in the attached patch.
          Hide
          Mike Matrigali added a comment -

          >t's been a while since I looked into upgrade and stored procedures. Does the restriction above come from the fact that we can't support >the alternative behavior? That would be to allow the procedure to be called on a soft upgraded database to remove orphaned statistics, >and then continue using the older version afterwards. This way the problem can be fixed without moving off the current Derby version.
          >Even if the above is technically possible, I'm fine with forbidding it if it conflicts with our current policy. I was just a bit curious

          I think it "might" be possible, but that we should not do it for soft upgrade. The idea with soft upgrade is that a user should be able to run 10.9 software in soft upgrade mode and then go back to running 10.1 software if it is a 10.1 database. If their application has calls to the new drop statistics routine it is very likely not to work when run in 10.1 software and we don't want to be supporting that. This specific routine might work for some subset of situation, but the policy to disable new syntax features in soft upgrade seems clean and leads
          to easily understood behavior, rather than allow some and not others.

          Show
          Mike Matrigali added a comment - >t's been a while since I looked into upgrade and stored procedures. Does the restriction above come from the fact that we can't support >the alternative behavior? That would be to allow the procedure to be called on a soft upgraded database to remove orphaned statistics, >and then continue using the older version afterwards. This way the problem can be fixed without moving off the current Derby version. >Even if the above is technically possible, I'm fine with forbidding it if it conflicts with our current policy. I was just a bit curious I think it "might" be possible, but that we should not do it for soft upgrade. The idea with soft upgrade is that a user should be able to run 10.9 software in soft upgrade mode and then go back to running 10.1 software if it is a 10.1 database. If their application has calls to the new drop statistics routine it is very likely not to work when run in 10.1 software and we don't want to be supporting that. This specific routine might work for some subset of situation, but the policy to disable new syntax features in soft upgrade seems clean and leads to easily understood behavior, rather than allow some and not others.
          Hide
          Mike Matrigali added a comment -

          I am good with the name of the procedure, and with respect to roles and permissions I think it should just function the same way as update statistics routine does.

          Show
          Mike Matrigali added a comment - I am good with the name of the procedure, and with respect to roles and permissions I think it should just function the same way as update statistics routine does.
          Hide
          Mamta A. Satoor added a comment -

          I am attaching second incremental patch(DERBY4115_patch2_diff) which takes care of some of the test failures(expected failures because of the addition of a new system procedure). I am rerunning the junit suite to make sure there is no other test failing because of the new stored procedure. I will also run derbyall to see if it has any problems. Thanks

          Show
          Mamta A. Satoor added a comment - I am attaching second incremental patch(DERBY4115_patch2_diff) which takes care of some of the test failures(expected failures because of the addition of a new system procedure). I am rerunning the junit suite to make sure there is no other test failing because of the new stored procedure. I will also run derbyall to see if it has any problems. Thanks
          Hide
          Mamta A. Satoor added a comment -

          I am attaching another patch(DERBY4115_patch3_diff) which is similar to patch 2 with the addition of basic upgrade test for the new procedure. This test ensures that drop statistics procedure is available only after hard upgrade.

          Show
          Mamta A. Satoor added a comment - I am attaching another patch(DERBY4115_patch3_diff) which is similar to patch 2 with the addition of basic upgrade test for the new procedure. This test ensures that drop statistics procedure is available only after hard upgrade.
          Hide
          Mamta A. Satoor added a comment -

          As the next step, I am looking at actually implementing the routine now that the ground work is done. I think the implementation should be similar to update statistics, ie allow the code to go through ALTER TABLE where permission/privilege checking, table/schema/index name validations happen automatically and we can implement the routine through ALTER TABLE. This will require changing ALTER TABLE syntax(same as we did for update statistics) but it will be an internal syntax and won't be available to an end user directly.

          Show
          Mamta A. Satoor added a comment - As the next step, I am looking at actually implementing the routine now that the ground work is done. I think the implementation should be similar to update statistics, ie allow the code to go through ALTER TABLE where permission/privilege checking, table/schema/index name validations happen automatically and we can implement the routine through ALTER TABLE. This will require changing ALTER TABLE syntax(same as we did for update statistics) but it will be an internal syntax and won't be available to an end user directly.
          Hide
          Mamta A. Satoor added a comment -

          Attaching patch, DERBY4115_patch4_diff, which is same as previous patch but now has changes to sqlgrammar.,jj to recognize the internal syntax ALTER TABLE tablename ALL DROP STATISTICS or ALTER TABLE tablename DROP STATISTICS indexname. Currently, this just calls the code for update statistics. This is of course wrong but I wanted to see that the basic framework to support new syntax works in the grammar.

          Next, I plan to change the new procedure to actually send the ALTER TABLE DROP STATISTICS call when the procedure is invoked.

          After thatt, I plan to make changes so that we call some dummy code(as the first step) in alter table when DROP STATISTICS is requested by the new stored procedure through the sql grammar..

          Show
          Mamta A. Satoor added a comment - Attaching patch, DERBY4115_patch4_diff, which is same as previous patch but now has changes to sqlgrammar.,jj to recognize the internal syntax ALTER TABLE tablename ALL DROP STATISTICS or ALTER TABLE tablename DROP STATISTICS indexname. Currently, this just calls the code for update statistics. This is of course wrong but I wanted to see that the basic framework to support new syntax works in the grammar. Next, I plan to change the new procedure to actually send the ALTER TABLE DROP STATISTICS call when the procedure is invoked. After thatt, I plan to make changes so that we call some dummy code(as the first step) in alter table when DROP STATISTICS is requested by the new stored procedure through the sql grammar..
          Hide
          Bryan Pendleton added a comment -

          Is this syntax just an example? Or is this the intended syntax? I'm trying to figure out what the word "ALL" means in "ALL DROP STATISTICS", and whether it should be "DROP ALL STATISTICS" instead.

          Show
          Bryan Pendleton added a comment - Is this syntax just an example? Or is this the intended syntax? I'm trying to figure out what the word "ALL" means in "ALL DROP STATISTICS", and whether it should be "DROP ALL STATISTICS" instead.
          Hide
          Mamta A. Satoor added a comment -

          Hi Bryan, thanks for looking at the patch.

          The syntax indeed is "ALL DROP STATISTICS". It pretty much mirrors what we have implemented for updated statistics which has the internal syntax of "ALL UPDATE STATISTICS". I am not sure why we chose it to be that way. May be it was easier to implement it that way in the parser and since this is internal only syntax, may be we decided to go ahead with "ALL UPDATE STATISTICS" rather than "UPDATE ALL STATISTICS".

          Show
          Mamta A. Satoor added a comment - Hi Bryan, thanks for looking at the patch. The syntax indeed is "ALL DROP STATISTICS". It pretty much mirrors what we have implemented for updated statistics which has the internal syntax of "ALL UPDATE STATISTICS". I am not sure why we chose it to be that way. May be it was easier to implement it that way in the parser and since this is internal only syntax, may be we decided to go ahead with "ALL UPDATE STATISTICS" rather than "UPDATE ALL STATISTICS".
          Hide
          Mamta A. Satoor added a comment -

          Attaching DERBY4115_patch5_diff, which is similar to earlier patch(DERBY4115_patch4_diff) but now has the new procedure actually calling the ALTER TABLE to drop the statistics rather than just ding a println. Additionally, I also had to change sqlgrammar.jj compared to the last patch. This is because of the parsing conflict that can arise when a table has a column named STATISTICS and an index named STATISTICS. If the user wants to drop the column STATISTICS, they will issue ALTER TABLE t1 DROP STATISITCS. If the user wants to drop the statistics for index STATISTICS, the patch 4(earlier patch) would generate ALTER TABLE DROP STATISITCS indexanme. This causes the parser to get confused and give an error. In order to solve this problem, without spending too much time on the syntax since it is an internal only syntax anyways(since a user will not be issue this ALTER TABLE syntax directly to drop the statisitcs), I have changed the grammar to use following internal syntax for dropping statisitcs ALTER TABLE STATISTICS DROP indexname

          So, following will be the internal syntax for various formats of update and drop statisitcs generated through SYSCS_UPDATE_STATISTICS and SYSCS_DROP_STATISTICS
          ALTER TABLE ALL DROP STATISITCS – when the user wants all the statistics for a table to be dropped
          ALTER TABLE STATISTICS DROP indexname - when the user wants only indexname's statistics dropped
          Corresponding update statistics syntax are as follows
          ALTER TABLE ALL UODATE STATISITCS – when the user wants all the statistics for a table to be updated
          ALTER TABLE UPDATE STATISTICS indexname - when the user wants only indexname's statistics updated

          For consistency purposes, I could change the update statistics single index to match the drop statistics single index as follows but first I want to focus on finishing the changes for drop statistics
          ALTER TABLE STATISTICS UPDATE indexname - when the user wants only indexname's statistics updated

          The details of all the changes in this patch are listed below.
          1)Added a new routine SYSCS_DROP_STATISTICS, with public access similar to SYSCS_UPDATE_STATISTICS. This happens in DataDictionaryImpl, where SYSCS_DROP_STATISTICS is added to the list of public access procedures in sysUtilProceduresWithPublicAccess
          2)The new stored procedure implementation is similar to update statistics, ie allow the routine to go through ALTER TABLE where permission/privilege checking, table/schema/index name validations happen automatically and we implement the routine logic through extension of ALTER TABLE syntax. This new syntax for ALTER TABLE syntax(same as we did for update statistics) is an internal syntax only and won't be available to an end user directly.
          3)This patch changes sqlgrammar.jj to recognize the following internal syntaxes for ALTER TABLE
          a)ALTER TABLE tablename ALL DROP STATISTICS
          The existing(corresponding syntax) for update statistics is as follows
          ALTER TABLE tablename ALL UPDATE STATISTICS
          b)ALTER TABLE tablename STATISTICS DROP indexname
          The existing(corresponding syntax) for update statistics is as follows
          ALTER TABLE tablename UPDATE STATISTICS indexname
          Notice the two syntaxes for index level statistics are different for drop vs update.(the reason for the syntax difference is explained above)
          4)The patch takes care of some of the test failures(expected failures because of the addition of a new system procedure).
          5)The patch adds basic upgrade test for the new procedure. This test ensures that drop statistics procedure is available only after hard upgrade.

          Next step to do is to add more tests(regular and upgrade tests). Add more comments in the code where appropriate.

          Future improvement step (which could go as patch of it's own) could be -
          For consistency purposes, we could change the update statistics single index to match the drop statistics single index but first I want to focus on finishing the changes for drop statistics

          Show
          Mamta A. Satoor added a comment - Attaching DERBY4115_patch5_diff, which is similar to earlier patch(DERBY4115_patch4_diff) but now has the new procedure actually calling the ALTER TABLE to drop the statistics rather than just ding a println. Additionally, I also had to change sqlgrammar.jj compared to the last patch. This is because of the parsing conflict that can arise when a table has a column named STATISTICS and an index named STATISTICS. If the user wants to drop the column STATISTICS, they will issue ALTER TABLE t1 DROP STATISITCS. If the user wants to drop the statistics for index STATISTICS, the patch 4(earlier patch) would generate ALTER TABLE DROP STATISITCS indexanme. This causes the parser to get confused and give an error. In order to solve this problem, without spending too much time on the syntax since it is an internal only syntax anyways(since a user will not be issue this ALTER TABLE syntax directly to drop the statisitcs), I have changed the grammar to use following internal syntax for dropping statisitcs ALTER TABLE STATISTICS DROP indexname So, following will be the internal syntax for various formats of update and drop statisitcs generated through SYSCS_UPDATE_STATISTICS and SYSCS_DROP_STATISTICS ALTER TABLE ALL DROP STATISITCS – when the user wants all the statistics for a table to be dropped ALTER TABLE STATISTICS DROP indexname - when the user wants only indexname's statistics dropped Corresponding update statistics syntax are as follows ALTER TABLE ALL UODATE STATISITCS – when the user wants all the statistics for a table to be updated ALTER TABLE UPDATE STATISTICS indexname - when the user wants only indexname's statistics updated For consistency purposes, I could change the update statistics single index to match the drop statistics single index as follows but first I want to focus on finishing the changes for drop statistics ALTER TABLE STATISTICS UPDATE indexname - when the user wants only indexname's statistics updated The details of all the changes in this patch are listed below. 1)Added a new routine SYSCS_DROP_STATISTICS, with public access similar to SYSCS_UPDATE_STATISTICS. This happens in DataDictionaryImpl, where SYSCS_DROP_STATISTICS is added to the list of public access procedures in sysUtilProceduresWithPublicAccess 2)The new stored procedure implementation is similar to update statistics, ie allow the routine to go through ALTER TABLE where permission/privilege checking, table/schema/index name validations happen automatically and we implement the routine logic through extension of ALTER TABLE syntax. This new syntax for ALTER TABLE syntax(same as we did for update statistics) is an internal syntax only and won't be available to an end user directly. 3)This patch changes sqlgrammar.jj to recognize the following internal syntaxes for ALTER TABLE a)ALTER TABLE tablename ALL DROP STATISTICS The existing(corresponding syntax) for update statistics is as follows ALTER TABLE tablename ALL UPDATE STATISTICS b)ALTER TABLE tablename STATISTICS DROP indexname The existing(corresponding syntax) for update statistics is as follows ALTER TABLE tablename UPDATE STATISTICS indexname Notice the two syntaxes for index level statistics are different for drop vs update.(the reason for the syntax difference is explained above) 4)The patch takes care of some of the test failures(expected failures because of the addition of a new system procedure). 5)The patch adds basic upgrade test for the new procedure. This test ensures that drop statistics procedure is available only after hard upgrade. Next step to do is to add more tests(regular and upgrade tests). Add more comments in the code where appropriate. Future improvement step (which could go as patch of it's own) could be - For consistency purposes, we could change the update statistics single index to match the drop statistics single index but first I want to focus on finishing the changes for drop statistics
          Hide
          Kristian Waagan added a comment -

          Hi Mamta,

          I had a look at patch 5. The tests ran sucessfully, and my simple manual invokations behaved like they should.
          Here are some comments, suggestions, and questions about the patch:
          a) Inconsistent use of tabs and spaces for indentation.
          b) Would it be okay to use (indexNameForStatistics == null) instead of
          dropStatisticsAll to reduce the state of the class? The relationship
          could be documented with dropStatistics.
          c) Typo in sqlgrammar.jj
          "By the type" -> "By the time"
          d) SystemProcedures.SYSCS_DROP_STATISTICS:
          o typo in @param indexname. Also, I don't understand the comment about
          indexname being the empty string. Is that allowed? The parser falls over
          if I try to specify it as an argument to the SQL call.
          o incorrect @exception (or @throws) in Javadoc
          o debug println
          e) What's the reason for the change in Changes10_2?

          Thanks,

          Show
          Kristian Waagan added a comment - Hi Mamta, I had a look at patch 5. The tests ran sucessfully, and my simple manual invokations behaved like they should. Here are some comments, suggestions, and questions about the patch: a) Inconsistent use of tabs and spaces for indentation. b) Would it be okay to use (indexNameForStatistics == null) instead of dropStatisticsAll to reduce the state of the class? The relationship could be documented with dropStatistics. c) Typo in sqlgrammar.jj "By the type" -> "By the time" d) SystemProcedures.SYSCS_DROP_STATISTICS: o typo in @param indexname. Also, I don't understand the comment about indexname being the empty string. Is that allowed? The parser falls over if I try to specify it as an argument to the SQL call. o incorrect @exception (or @throws) in Javadoc o debug println e) What's the reason for the change in Changes10_2? Thanks,
          Hide
          Mamta A. Satoor added a comment -

          Kristian, thanks for taking the time to review the patch. Here are my answers.
          a) Inconsistent use of tabs and spaces for indentation.
          I will fix this. Not sure why Eclipse is not consistent in using one vs the others.

          b) Would it be okay to use (indexNameForStatistics == null) instead of
          dropStatisticsAll to reduce the state of the class? The relationship
          could be documented with dropStatistics.
          I wondered about this myself when I copied the model for update statistics for drop statistics. My gut feeling is that this was done in order to avoid the conflict with pre-existing 4 parameter init() method for compress table and hence we may have decided to add a new init method with 5 parameters but I am not 100% sure about this theory. I am planning on uploading a patch soon which will update upgrade tests and more regular tests for drop statistics. If there are no comments on that patch, I would like to go ahead and commit that patch. We can address the redundancy of extra parameter dropStatisticsAll/updateStatisticsAll in a future patch.

          c) Typo in sqlgrammar.jj
          "By the type" -> "By the time"
          I will fix this.

          d) SystemProcedures.SYSCS_DROP_STATISTICS:
          o typo in @param indexname. Also, I don't understand the comment about
          indexname being the empty string. Is that allowed? The parser falls over
          if I try to specify it as an argument to the SQL call.
          o incorrect @exception (or @throws) in Javadoc
          o debug println
          I will fix this.

          e) What's the reason for the change in Changes10_2?
          The test after hard upgrades seem to check how many system procedures have public access. Since the drop statistics procedure has public access(similar to update statistics), we need to include that in the expected resultset list.

          Show
          Mamta A. Satoor added a comment - Kristian, thanks for taking the time to review the patch. Here are my answers. a) Inconsistent use of tabs and spaces for indentation. I will fix this. Not sure why Eclipse is not consistent in using one vs the others. b) Would it be okay to use (indexNameForStatistics == null) instead of dropStatisticsAll to reduce the state of the class? The relationship could be documented with dropStatistics. I wondered about this myself when I copied the model for update statistics for drop statistics. My gut feeling is that this was done in order to avoid the conflict with pre-existing 4 parameter init() method for compress table and hence we may have decided to add a new init method with 5 parameters but I am not 100% sure about this theory. I am planning on uploading a patch soon which will update upgrade tests and more regular tests for drop statistics. If there are no comments on that patch, I would like to go ahead and commit that patch. We can address the redundancy of extra parameter dropStatisticsAll/updateStatisticsAll in a future patch. c) Typo in sqlgrammar.jj "By the type" -> "By the time" I will fix this. d) SystemProcedures.SYSCS_DROP_STATISTICS: o typo in @param indexname. Also, I don't understand the comment about indexname being the empty string. Is that allowed? The parser falls over if I try to specify it as an argument to the SQL call. o incorrect @exception (or @throws) in Javadoc o debug println I will fix this. e) What's the reason for the change in Changes10_2? The test after hard upgrades seem to check how many system procedures have public access. Since the drop statistics procedure has public access(similar to update statistics), we need to include that in the expected resultset list.
          Hide
          Mamta A. Satoor added a comment -

          Kristian, as for your following comment
          d) SystemProcedures.SYSCS_DROP_STATISTICS:
          o Also, I don't understand the comment about
          indexname being the empty string. Is that allowed? The parser falls over
          if I try to specify it as an argument to the SQL call.

          You are right, that giving an empty string makes the parse give following exception. I tried using empty string with compress table/update statistics procedure and saw the same behavior. I will go ahead and file a jira for that issue in general.
          ij> call syscs_util.SYSCS_DROP_STATISTICS( 'APP', 'T1', '' );
          ERROR 38000: The exception 'java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40.' was thrown while evaluating an expression.
          java.sql.SQLException: The exception 'java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40.' was thrown while evaluating an expression.
          at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:98)
          at org.apache.derby.impl.jdbc.Util.newEmbedSQLException(Util.java:142)
          at org.apache.derby.impl.jdbc.Util.seeNextException(Util.java:278)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:431)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:353)
          at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2360)
          at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:82)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1334)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:630)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:559)
          at org.apache.derby.impl.tools.ij.ij.executeImmediate(ij.java:367)
          at org.apache.derby.impl.tools.ij.utilMain.doCatch(utilMain.java:527)
          at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(utilMain.java:369)
          at org.apache.derby.impl.tools.ij.utilMain.go(utilMain.java:245)
          at org.apache.derby.impl.tools.ij.Main.go(Main.java:229)
          at org.apache.derby.impl.tools.ij.Main.mainCore(Main.java:184)
          at org.apache.derby.impl.tools.ij.Main.main(Main.java:75)
          at org.apache.derby.tools.ij.main(ij.java:59)
          Caused by: java.sql.SQLException: The exception 'java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40.' was thrown while evaluating an expression.
          at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:42)
          at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(SQLExceptionFactory40.java:122)
          at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:71)
          ... 17 more
          Caused by: java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40.
          at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:42)
          at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(SQLExceptionFactory40.java:122)
          at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:71)
          at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Util.java:256)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:424)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:353)
          at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2360)
          at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:82)
          at org.apache.derby.impl.jdbc.EmbedPreparedStatement.<init>(EmbedPreparedStatement.java:153)
          at org.apache.derby.jdbc.Driver40.newEmbedPreparedStatement(Driver40.java:107)
          at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(EmbedConnection.java:1685)
          at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(EmbedConnection.java:1513)
          at org.apache.derby.catalog.SystemProcedures.SYSCS_DROP_STATISTICS(SystemProcedures.java:792)
          at org.apache.derby.exe.acace4c0a3x0137x2f19xcc22x00000013e5701.g0(Unknown Source)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:60)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37)
          at java.lang.reflect.Method.invoke(Method.java:611)
          at org.apache.derby.impl.services.reflect.ReflectMethod.invoke(ReflectMethod.java:46)
          at org.apache.derby.impl.sql.execute.CallStatementResultSet.open(CallStatementResultSet.java:75)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:443)
          at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:324)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1242)
          ... 10 more
          Caused by: ERROR 42X01: Syntax error: Encountered "\"" at line 1, column 40.
          at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:278)
          at org.apache.derby.impl.sql.compile.ParserImpl.parseStatement(ParserImpl.java:153)
          at org.apache.derby.impl.sql.GenericStatement.prepMinion(GenericStatement.java:357)
          at org.apache.derby.impl.sql.GenericStatement.prepare(GenericStatement.java:99)
          at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement(GenericLanguageConnectionContext.java:1103)
          at org.apache.derby.impl.jdbc.EmbedPreparedStatement.<init>(EmbedPreparedStatement.java:134)
          ... 24 more
          ERROR 42X01: Syntax error: Encountered "\"" at line 1, column 40.
          java.sql.SQLSyntaxErrorException: Syntax error: Encountered "\"" at line 1, column 40.
          at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:92)
          at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Util.java:256)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:424)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:353)
          at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2360)
          at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:82)
          at org.apache.derby.impl.jdbc.EmbedPreparedStatement.<init>(EmbedPreparedStatement.java:153)
          at org.apache.derby.jdbc.Driver40.newEmbedPreparedStatement(Driver40.java:107)
          at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(EmbedConnection.java:1685)
          at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(EmbedConnection.java:1513)
          at org.apache.derby.catalog.SystemProcedures.SYSCS_DROP_STATISTICS(SystemProcedures.java:792)
          at org.apache.derby.exe.acace4c0a3x0137x2f19xcc22x00000013e5701.g0(Unknown Source)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:60)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37)
          at java.lang.reflect.Method.invoke(Method.java:611)
          at org.apache.derby.impl.services.reflect.ReflectMethod.invoke(ReflectMethod.java:46)
          at org.apache.derby.impl.sql.execute.CallStatementResultSet.open(CallStatementResultSet.java:75)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:443)
          at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:324)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1242)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:630)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:559)
          at org.apache.derby.impl.tools.ij.ij.executeImmediate(ij.java:367)
          at org.apache.derby.impl.tools.ij.utilMain.doCatch(utilMain.java:527)
          at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(utilMain.java:369)
          at org.apache.derby.impl.tools.ij.utilMain.go(utilMain.java:245)
          at org.apache.derby.impl.tools.ij.Main.go(Main.java:229)
          at org.apache.derby.impl.tools.ij.Main.mainCore(Main.java:184)
          at org.apache.derby.impl.tools.ij.Main.main(Main.java:75)
          at org.apache.derby.tools.ij.main(ij.java:59)
          Caused by: java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40.
          at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:42)
          at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(SQLExceptionFactory40.java:122)
          at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:71)
          ... 30 more
          Caused by: ERROR 42X01: Syntax error: Encountered "\"" at line 1, column 40.
          at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:278)
          at org.apache.derby.impl.sql.compile.ParserImpl.parseStatement(ParserImpl.java:153)
          at org.apache.derby.impl.sql.GenericStatement.prepMinion(GenericStatement.java:357)
          at org.apache.derby.impl.sql.GenericStatement.prepare(GenericStatement.java:99)
          at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement(GenericLanguageConnectionContext.java:1103)
          at org.apache.derby.impl.jdbc.EmbedPreparedStatement.<init>(EmbedPreparedStatement.java:134)
          ... 24 more
          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
          Mamta A. Satoor added a comment - Kristian, as for your following comment d) SystemProcedures.SYSCS_DROP_STATISTICS: o Also, I don't understand the comment about indexname being the empty string. Is that allowed? The parser falls over if I try to specify it as an argument to the SQL call. You are right, that giving an empty string makes the parse give following exception. I tried using empty string with compress table/update statistics procedure and saw the same behavior. I will go ahead and file a jira for that issue in general. ij> call syscs_util.SYSCS_DROP_STATISTICS( 'APP', 'T1', '' ); ERROR 38000: The exception 'java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40.' was thrown while evaluating an expression. java.sql.SQLException: The exception 'java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40.' was thrown while evaluating an expression. at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:98) at org.apache.derby.impl.jdbc.Util.newEmbedSQLException(Util.java:142) at org.apache.derby.impl.jdbc.Util.seeNextException(Util.java:278) at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:431) at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:353) at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2360) at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:82) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1334) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:630) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:559) at org.apache.derby.impl.tools.ij.ij.executeImmediate(ij.java:367) at org.apache.derby.impl.tools.ij.utilMain.doCatch(utilMain.java:527) at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(utilMain.java:369) at org.apache.derby.impl.tools.ij.utilMain.go(utilMain.java:245) at org.apache.derby.impl.tools.ij.Main.go(Main.java:229) at org.apache.derby.impl.tools.ij.Main.mainCore(Main.java:184) at org.apache.derby.impl.tools.ij.Main.main(Main.java:75) at org.apache.derby.tools.ij.main(ij.java:59) Caused by: java.sql.SQLException: The exception 'java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40.' was thrown while evaluating an expression. at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:42) at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(SQLExceptionFactory40.java:122) at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:71) ... 17 more Caused by: java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40. at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:42) at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(SQLExceptionFactory40.java:122) at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:71) at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Util.java:256) at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:424) at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:353) at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2360) at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:82) at org.apache.derby.impl.jdbc.EmbedPreparedStatement.<init>(EmbedPreparedStatement.java:153) at org.apache.derby.jdbc.Driver40.newEmbedPreparedStatement(Driver40.java:107) at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(EmbedConnection.java:1685) at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(EmbedConnection.java:1513) at org.apache.derby.catalog.SystemProcedures.SYSCS_DROP_STATISTICS(SystemProcedures.java:792) at org.apache.derby.exe.acace4c0a3x0137x2f19xcc22x00000013e5701.g0(Unknown Source) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:60) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37) at java.lang.reflect.Method.invoke(Method.java:611) at org.apache.derby.impl.services.reflect.ReflectMethod.invoke(ReflectMethod.java:46) at org.apache.derby.impl.sql.execute.CallStatementResultSet.open(CallStatementResultSet.java:75) at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:443) at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:324) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1242) ... 10 more Caused by: ERROR 42X01: Syntax error: Encountered "\"" at line 1, column 40. at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:278) at org.apache.derby.impl.sql.compile.ParserImpl.parseStatement(ParserImpl.java:153) at org.apache.derby.impl.sql.GenericStatement.prepMinion(GenericStatement.java:357) at org.apache.derby.impl.sql.GenericStatement.prepare(GenericStatement.java:99) at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement(GenericLanguageConnectionContext.java:1103) at org.apache.derby.impl.jdbc.EmbedPreparedStatement.<init>(EmbedPreparedStatement.java:134) ... 24 more ERROR 42X01: Syntax error: Encountered "\"" at line 1, column 40. java.sql.SQLSyntaxErrorException: Syntax error: Encountered "\"" at line 1, column 40. at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:92) at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Util.java:256) at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:424) at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:353) at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2360) at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:82) at org.apache.derby.impl.jdbc.EmbedPreparedStatement.<init>(EmbedPreparedStatement.java:153) at org.apache.derby.jdbc.Driver40.newEmbedPreparedStatement(Driver40.java:107) at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(EmbedConnection.java:1685) at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(EmbedConnection.java:1513) at org.apache.derby.catalog.SystemProcedures.SYSCS_DROP_STATISTICS(SystemProcedures.java:792) at org.apache.derby.exe.acace4c0a3x0137x2f19xcc22x00000013e5701.g0(Unknown Source) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:60) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37) at java.lang.reflect.Method.invoke(Method.java:611) at org.apache.derby.impl.services.reflect.ReflectMethod.invoke(ReflectMethod.java:46) at org.apache.derby.impl.sql.execute.CallStatementResultSet.open(CallStatementResultSet.java:75) at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:443) at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:324) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1242) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:630) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:559) at org.apache.derby.impl.tools.ij.ij.executeImmediate(ij.java:367) at org.apache.derby.impl.tools.ij.utilMain.doCatch(utilMain.java:527) at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(utilMain.java:369) at org.apache.derby.impl.tools.ij.utilMain.go(utilMain.java:245) at org.apache.derby.impl.tools.ij.Main.go(Main.java:229) at org.apache.derby.impl.tools.ij.Main.mainCore(Main.java:184) at org.apache.derby.impl.tools.ij.Main.main(Main.java:75) at org.apache.derby.tools.ij.main(ij.java:59) Caused by: java.sql.SQLException: Syntax error: Encountered "\"" at line 1, column 40. at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:42) at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(SQLExceptionFactory40.java:122) at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:71) ... 30 more Caused by: ERROR 42X01: Syntax error: Encountered "\"" at line 1, column 40. at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:278) at org.apache.derby.impl.sql.compile.ParserImpl.parseStatement(ParserImpl.java:153) at org.apache.derby.impl.sql.GenericStatement.prepMinion(GenericStatement.java:357) at org.apache.derby.impl.sql.GenericStatement.prepare(GenericStatement.java:99) at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement(GenericLanguageConnectionContext.java:1103) at org.apache.derby.impl.jdbc.EmbedPreparedStatement.<init>(EmbedPreparedStatement.java:134) ... 24 more 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
          Mamta A. Satoor added a comment -

          Created following jira for exception thrown when empty string is passed for index name to update/drop statistics procedures
          DERBY-5750 Sending an empty string as table name to compress table procedure or empty string as index name to update statistics procedure makes the parser throw an exception.

          Show
          Mamta A. Satoor added a comment - Created following jira for exception thrown when empty string is passed for index name to update/drop statistics procedures DERBY-5750 Sending an empty string as table name to compress table procedure or empty string as index name to update statistics procedure makes the parser throw an exception.
          Hide
          Mamta A. Satoor added a comment -

          Attaching another patch, DERBY4115_patch6_diff.txt, which has more tests including upgrade tests. Additionally, it also fixes code in AlterTableConstantAction to make sure that it sends invalidation signal to dependent statements when statistics are dropped on a table. I found this bug in my previous patch while trying to write a test where the query plan chosen for the query should have changed after the statistics were dropped.

          While writing the upgrade tests, I found that a meaningful test for drop statistics could only be written for Derby releases 10.5 and higher. We have found that when constraints end up sharing same backing index, Derby won't create statistics for them. This is issue DERBY-5702. But if we run update statistics on that constraint, we will be able to get the statistics for such a constraint. Later, when the constraint is dropped, because of DERBY-5681, the statistics row for such a constraint(one that shares it's backing index with another constraint) is never dropped. We can use drop statistics procedure introduced in this jira to take care of such hanging indexes. But since update statistics procedure is only available in 10.5 and higher, I couldn't demonstrate use of drop statistics to drop hanging statistics rows.

          I have also taken care of some of the review comments by Kristian.

          The details of all the changes in this patch are listed below.
          1)Added a new routine SYSCS_DROP_STATISTICS, with public access similar to SYSCS_UPDATE_STATISTICS. This happens in DataDictionaryImpl, where SYSCS_DROP_STATISTICS is added to the list of public access procedures in sysUtilProceduresWithPublicAccess
          2)The new stored procedure implementation is similar to update statistics, ie allow the routine to go through ALTER TABLE where permission/privilege checking, table/schema/index name validations happen automatically and we implement the routine logic through extension of ALTER TABLE syntax. This new syntax for ALTER TABLE syntax(same as we did for update statistics) is an internal syntax only and won't be available to an end user directly.
          3)This patch changes sqlgrammar.jj to recognize the following internal syntaxes for ALTER TABLE
          a)ALTER TABLE tablename ALL DROP STATISTICS
          The existing(corresponding syntax) for update statistics is as follows
          ALTER TABLE tablename ALL UPDATE STATISTICS
          b)ALTER TABLE tablename STATISTICS DROP indexname
          The existing(corresponding syntax) for update statistics is as follows
          ALTER TABLE tablename UPDATE STATISTICS indexname
          Notice the two syntaxes for index level statistics are different for drop vs update.(the reason for the syntax difference is explained above)
          4)After the statistics are dropped, we send invalidation signal to dependent statements so they would get recompiled when they are executed next time. This will make sure that they pick the correct plan given the statistics for the table.
          5)The patch takes care of some of the test failures(expected failures because of the addition of a new system procedure).
          6)The patch adds basic upgrade test for the new procedure. This test ensures that drop statistics procedure is available only after hard upgrade.
          7)While writing the upgrade tests, I found that a meaningful test for drop statistics could only be written for Derby releases 10.5 and higher. We have found that when constraints end up sharing same backing index, Derby won't create statistics for them. This is issue DERBY-5702. But if we run update statistics on that constraint, we will be able to get the statistics for such a constraint. Later, when the constraint is dropped, because of DERBY-5681, the statistics row for such a constraint(one that shares it's backing index with another constraint) is never dropped. We can use drop statistics procedure introduced in this jira to take care of such hanging indexes. But since update statistics procedure is only available in 10.5 and higher, I couldn't demonstrate use of drop statistics to drop hanging statistics rows.

          If there are no further comments on this patch, I will work on the committing this patch tomorrow. Thanks

          Show
          Mamta A. Satoor added a comment - Attaching another patch, DERBY4115_patch6_diff.txt, which has more tests including upgrade tests. Additionally, it also fixes code in AlterTableConstantAction to make sure that it sends invalidation signal to dependent statements when statistics are dropped on a table. I found this bug in my previous patch while trying to write a test where the query plan chosen for the query should have changed after the statistics were dropped. While writing the upgrade tests, I found that a meaningful test for drop statistics could only be written for Derby releases 10.5 and higher. We have found that when constraints end up sharing same backing index, Derby won't create statistics for them. This is issue DERBY-5702 . But if we run update statistics on that constraint, we will be able to get the statistics for such a constraint. Later, when the constraint is dropped, because of DERBY-5681 , the statistics row for such a constraint(one that shares it's backing index with another constraint) is never dropped. We can use drop statistics procedure introduced in this jira to take care of such hanging indexes. But since update statistics procedure is only available in 10.5 and higher, I couldn't demonstrate use of drop statistics to drop hanging statistics rows. I have also taken care of some of the review comments by Kristian. The details of all the changes in this patch are listed below. 1)Added a new routine SYSCS_DROP_STATISTICS, with public access similar to SYSCS_UPDATE_STATISTICS. This happens in DataDictionaryImpl, where SYSCS_DROP_STATISTICS is added to the list of public access procedures in sysUtilProceduresWithPublicAccess 2)The new stored procedure implementation is similar to update statistics, ie allow the routine to go through ALTER TABLE where permission/privilege checking, table/schema/index name validations happen automatically and we implement the routine logic through extension of ALTER TABLE syntax. This new syntax for ALTER TABLE syntax(same as we did for update statistics) is an internal syntax only and won't be available to an end user directly. 3)This patch changes sqlgrammar.jj to recognize the following internal syntaxes for ALTER TABLE a)ALTER TABLE tablename ALL DROP STATISTICS The existing(corresponding syntax) for update statistics is as follows ALTER TABLE tablename ALL UPDATE STATISTICS b)ALTER TABLE tablename STATISTICS DROP indexname The existing(corresponding syntax) for update statistics is as follows ALTER TABLE tablename UPDATE STATISTICS indexname Notice the two syntaxes for index level statistics are different for drop vs update.(the reason for the syntax difference is explained above) 4)After the statistics are dropped, we send invalidation signal to dependent statements so they would get recompiled when they are executed next time. This will make sure that they pick the correct plan given the statistics for the table. 5)The patch takes care of some of the test failures(expected failures because of the addition of a new system procedure). 6)The patch adds basic upgrade test for the new procedure. This test ensures that drop statistics procedure is available only after hard upgrade. 7)While writing the upgrade tests, I found that a meaningful test for drop statistics could only be written for Derby releases 10.5 and higher. We have found that when constraints end up sharing same backing index, Derby won't create statistics for them. This is issue DERBY-5702 . But if we run update statistics on that constraint, we will be able to get the statistics for such a constraint. Later, when the constraint is dropped, because of DERBY-5681 , the statistics row for such a constraint(one that shares it's backing index with another constraint) is never dropped. We can use drop statistics procedure introduced in this jira to take care of such hanging indexes. But since update statistics procedure is only available in 10.5 and higher, I couldn't demonstrate use of drop statistics to drop hanging statistics rows. If there are no further comments on this patch, I will work on the committing this patch tomorrow. Thanks
          Hide
          Mamta A. Satoor added a comment - - edited

          This issue can't be backported because of the changes to the contents of the system tables.

          Show
          Mamta A. Satoor added a comment - - edited This issue can't be backported because of the changes to the contents of the system tables.
          Hide
          Kristian Waagan added a comment -

          Attaching patch 7a, which moves the upgrade test from BasicSetup to Changes10_9. With the changes going in for DERBY-3790 there are more states to consider, and it makes more sense to move the test such that you don't have to consider all states (i.e. the expected number of statistics entries).
          I also corrected a bug which would fail if the test ever ran on 10.9 (it expected to fail when trying to invoke SYSCS_DROP_STATISTICS, but this would have suceeded on 10.9).

          The upgrade tests passed for 10.1.1.0 and 10.8.2.2.
          I expect to commit this patch shortly, since I have basically only moved code (I also removed some ifs).
          Patch ready for review.

          Show
          Kristian Waagan added a comment - Attaching patch 7a, which moves the upgrade test from BasicSetup to Changes10_9. With the changes going in for DERBY-3790 there are more states to consider, and it makes more sense to move the test such that you don't have to consider all states (i.e. the expected number of statistics entries). I also corrected a bug which would fail if the test ever ran on 10.9 (it expected to fail when trying to invoke SYSCS_DROP_STATISTICS, but this would have suceeded on 10.9). The upgrade tests passed for 10.1.1.0 and 10.8.2.2. I expect to commit this patch shortly, since I have basically only moved code (I also removed some ifs). Patch ready for review.
          Hide
          Kristian Waagan added a comment -

          suites.All completed with patch 7a.
          Committed to trunk with revision 1341059.

          Show
          Kristian Waagan added a comment - suites.All completed with patch 7a. Committed to trunk with revision 1341059.
          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.

            People

            • Assignee:
              Mamta A. Satoor
              Reporter:
              Kathey Marsden
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development