Uploaded image for project: 'Teiid'
  1. Teiid
  2. TEIID-2876

Order of DataTypeManager type names affects order of functions in FunctionLibrary

    XMLWordPrintable

Details

    • Bug
    • Resolution: Done
    • Minor
    • 8.7
    • 7.7, 8.4, 8.7
    • Query Engine
    • None
    • Hide

      Swap the addition to DataTypeNames of the data types Object and String in the static function of DataTypeManager and run the unit test TestFunctionResolving.testResolveCoalesce4(). If will fail with the above error.

      Show
      Swap the addition to DataTypeNames of the data types Object and String in the static function of DataTypeManager and run the unit test TestFunctionResolving.testResolveCoalesce4(). If will fail with the above error.

    Description

      This may be by design but the source code does not document this to be the case and appears brittle and open to future regressions.

      Executing the unit tests in TestFunctionResolving class, 'testResolveCoalesce4', the designer version of the teiid runtime client failed with the following error:

      java.lang.AssertionError: expected:<class java.lang.String> but was:<class java.lang.Object>
      

      The unit test passes in Teiid so I needed to determine the reason for the difference. Turns out the difference was the use of a LinkedHashSet rather than a HashSet.

      Tracing this back showed the following:

      1. In FunctionLibrary.determineNecessaryConversions, the functionMethods are iterated through. In the case of teiid, the String version of 'coalesce' appears before the Object version and despite both versions of the function being scored the same, the String version is preferred. In the case of designer's runtime client, the Object version appeared before the String version hence the former was chosen and the unit test failed.
      2. The functionMethods originate from the FunctionTree and are added into a TreeMap which is only ordered on the function name, ie. coalesce. The typed versions of this function are added to an ArrayList which is not sorted (halfway down addFunction()).
      3. The coalesce functions originate from the SystemSource class and are added by iterating over dataTypeManager.getAllDataTypeNames(). Since the latter are in a LinkedHashSet (in teiid) then ordering is preserved and the String datatype happens to precede the Object datatype. In the designer runtime client, this was a HashSet hence the difference in ordering and failing of the test.

      The consequence of this is that determineNecessaryConversions in the FunctionLibrary gets the correct answer for the unit test but seems rather brittle in that a small change to the DataTypeManager's order of DataTypeName additions could have an indirect and non-obvious effect on which typed version of a function is returned by the resolver.

      Suggestions:

      • Ensure that the ArrayLists of functions in the FunctionTree's functionByName map are sorted based upon a narrow to broad ordering of types, ie. Object as the most broad will always be at the end. This will ensure that the scoring in FunctionLibrary.determineNecessaryConversions() will in the event of a tie always pick the narrowest of typed function.
      • Change the scoring in determineNecessaryConversions() to account for 'equal to' as well as 'less than' since String and Object functions get the same score.
      • Document that the ordering of DataTypeManager data type names should not be changed without consideration of this scenario.

      Attachments

        Activity

          People

            rhn-engineering-shawkins Steven Hawkins
            parichar@redhat.com Paul Richardson
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: