Derby
  1. Derby
  2. DERBY-5755

Minor cleanup of DataDictionaryImpl.getRoutineList()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 10.9.1.0
    • Fix Version/s: 10.9.1.0
    • Component/s: SQL
    • Labels:
      None

      Description

      I noticed two small possibilities for improvement in DataDictionaryImpl.getRoutineList() when I worked on DERBY-5730:

      • The method always returns a list of one or zero items. Instead of creating an ArrayList, it should use Collections.singletonList() and Collections.EMPTY_LIST, which are more memory-efficient.
      • It loops through the entire SYSFUN_FUNCTIONS array, even if the matching function is found in the first cell of the array. It should break out of the loop once a match is found.
      1. d5755-1a.diff
        2 kB
        Knut Anders Hatlen
      2. d5755-1b.diff
        2 kB
        Knut Anders Hatlen

        Activity

        Hide
        Knut Anders Hatlen added a comment -

        Attaching a patch that makes the suggested changes to the method. All the regression tests passed with the patch.

        Show
        Knut Anders Hatlen added a comment - Attaching a patch that makes the suggested changes to the method. All the regression tests passed with the patch.
        Hide
        Dag H. Wanvik added a comment -

        Looks to me as if the calling code is checking against duplicates? Cf. PrivilegeNode ca line 105 which throws if mor ethan one routine returned. Wouldn't the new code mask this error checking?
        Cf also signature of getRoutineList: "Get the list of routines matching the schema and routine name" having a plural "routines".

        Show
        Dag H. Wanvik added a comment - Looks to me as if the calling code is checking against duplicates? Cf. PrivilegeNode ca line 105 which throws if mor ethan one routine returned. Wouldn't the new code mask this error checking? Cf also signature of getRoutineList: "Get the list of routines matching the schema and routine name" having a plural "routines".
        Hide
        Knut Anders Hatlen added a comment -

        Hi Dag,

        You're right that the interface (DataDictionary) allows for returning lists with more than one element, so the callers still need to account for that possibility, even though the specific implementation (DataDictionaryImpl) says in its javadoc that it always returns 0 or 1.

        As to the possibility of masking errors, you're right that the patch would mask duplicates in the SYSFUN_FUNCTIONS array, but it would not change anything for routines looked up in the catalogs, as the signature of getAliasDescriptor() guarantees that no more than one descriptor is returned. I'm not sure if we'll ever want to add duplicate names in SYSFUN_FUNCTIONS, but it might make sense to preserve the current code's readiness for it.

        What about this alternative patch (1b)? It makes the following changes:

        • remove the early termination of the loop that the 1a patch introduced
        • still collect results into an ArrayList, but only for SYSFUN_FUNCTIONS
        • use Collections.EMPTY_LIST and Collection.singletonList(), but only for the descriptors fetched from the catalog, which are known at compile-time to be exactly 0 or 1
        Show
        Knut Anders Hatlen added a comment - Hi Dag, You're right that the interface (DataDictionary) allows for returning lists with more than one element, so the callers still need to account for that possibility, even though the specific implementation (DataDictionaryImpl) says in its javadoc that it always returns 0 or 1. As to the possibility of masking errors, you're right that the patch would mask duplicates in the SYSFUN_FUNCTIONS array, but it would not change anything for routines looked up in the catalogs, as the signature of getAliasDescriptor() guarantees that no more than one descriptor is returned. I'm not sure if we'll ever want to add duplicate names in SYSFUN_FUNCTIONS, but it might make sense to preserve the current code's readiness for it. What about this alternative patch (1b)? It makes the following changes: remove the early termination of the loop that the 1a patch introduced still collect results into an ArrayList, but only for SYSFUN_FUNCTIONS use Collections.EMPTY_LIST and Collection.singletonList(), but only for the descriptors fetched from the catalog, which are known at compile-time to be exactly 0 or 1
        Hide
        Dag H. Wanvik added a comment -

        Thanks for the explanation. +1

        Show
        Dag H. Wanvik added a comment - Thanks for the explanation. +1
        Hide
        Knut Anders Hatlen added a comment -

        Thanks, Dag!
        Committed revision 1338167.

        Show
        Knut Anders Hatlen added a comment - Thanks, Dag! Committed revision 1338167.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development