Uploaded image for project: 'Derby'
  1. Derby
  2. DERBY-3494

Move the setup of NormalizeResultSetNode into the NormalizeResultSetNode



    • Improvement
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • SQL
    • None
    • Patch Available


      In DERBY-3310 Dan suggested ...

      Setting up a NormalizeResultSetNode is spread over three locations, the class itself (very little, it's almost acting like a C struct),
      the genNormalizeResultSetNode method and then copyLengthsAndTypesToSource. A good O-O implementation would have
      the logic to create a NormalizeResultSetNode self-contained in NormalizeResultSetNode.

      Since the ResultColumnList of the original ResultSetNode correctly describes the desired outcome, it's not clear to
      me why NormalizeResultSetNode can't just refer to the same list and use it for its processing. They may be some chance
      that this would cause recursion at some point, where a NormalizeResultSetNode would think it needed to be wrapped
      in a NormalizeResultSetNode since the types of its columns and expression don't match (i.e. when it is handled as a regular ResultSetNode).

      I think moving the setup of a NormalizeResultSetNode into the class itself, so that its inputs are just the ResultSetNode to wrap
      would help clear up the code, especially if comments were added indicating why certain actions were being taken.

      I am separating this task out into a separate issue, so that it can be worked on independently of DERBY-3310.


        1. npe.sql
          2 kB
          Katherine Marsden
        2. derby-3494_removeCopyTypesAndLengths_diff.txt
          4 kB
          Katherine Marsden
        3. derby-3494_remove_genNormalizeResultSetNode_diff.txt
          9 kB
          Katherine Marsden
        4. decompile.out
          15 kB
          Katherine Marsden
        5. d3494_npe_writeup.html
          35 kB
          A B
        6. d3494_npe_writeup.html
          35 kB
          A B

        Issue Links



              kmarsden Katherine Marsden
              kmarsden Katherine Marsden
              0 Vote for this issue
              0 Start watching this issue