Derby
  1. Derby
  2. DERBY-5246

Simplify bytecode generation for concatenation operator

    Details

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

      Description

      ConcatenationOperatorNode generates bytecode that ensures the result object is not null before calling the method that implements the operator. This breaks the pattern used by other operators (which ensures that the result object is not null inside the method that implements the operator, not in the generated bytecode), and it unnecessarily complicates the code in BinaryOperatorNode.

      The comments indicate that the current approach was chosen to prevent the null check from happening at execution time, but the generated bytecode does perform the null check at execution time, so generating byte code for it shouldn't have any real benefit over writing Java code for it. In general, implementing as much as possible of the execution time code as Java code is preferred to implementing it directly as bytecode because it's easier to read and debug Java code, and because the generated bytecode cannot be shared between execution plans and take more memory.

      1. concat.diff
        5 kB
        Knut Anders Hatlen

        Activity

        Knut Anders Hatlen created issue -
        Hide
        Knut Anders Hatlen added a comment -

        The attached patch removes the generation of the null check in ConcatenationOperatorNode and removes the special handling of concatenation that was required in BinaryOperatorNode because of it.

        The patch also changes the implementations of concatenate() in SQLChar, SQLLongvarchar and SQLBinary to work if a null value is passed in for the result object. This makes the methods match the javadoc comments in the interfaces (StringDataValue and BitDataValue), which state that the result parameter in the concatenate() methods should accept null values.

        The regression tests ran cleanly with the patch.

        Show
        Knut Anders Hatlen added a comment - The attached patch removes the generation of the null check in ConcatenationOperatorNode and removes the special handling of concatenation that was required in BinaryOperatorNode because of it. The patch also changes the implementations of concatenate() in SQLChar, SQLLongvarchar and SQLBinary to work if a null value is passed in for the result object. This makes the methods match the javadoc comments in the interfaces (StringDataValue and BitDataValue), which state that the result parameter in the concatenate() methods should accept null values. The regression tests ran cleanly with the patch.
        Knut Anders Hatlen made changes -
        Field Original Value New Value
        Attachment concat.diff [ 12480395 ]
        Knut Anders Hatlen made changes -
        Issue & fix info [Patch Available]
        Knut Anders Hatlen made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Knut Anders Hatlen added a comment -

        Committed revision 1127886.

        Show
        Knut Anders Hatlen added a comment - Committed revision 1127886.
        Knut Anders Hatlen made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Issue & fix info [Patch Available]
        Fix Version/s 10.9.0.0 [ 12316344 ]
        Resolution Fixed [ 1 ]
        Knut Anders Hatlen made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Gavin made changes -
        Workflow jira [ 12614600 ] Default workflow, editable Closed status [ 12801127 ]

          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