Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-2714

Make BasicSqlType immutable, and now createWithNullability re-uses existing type if possible

    XMLWordPrintableJSON

    Details

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

      Description

      As it is done in the parent implementation RelDataTypeFactoryImpl#createTypeWithNullability where the parameter 'type' is directly used as 'newType' if nullable matches:

      public RelDataType createTypeWithNullability(final RelDataType type, final boolean nullable) {
        RelDataType newType;
        if (type.isNullable() == nullable) {
          newType = type;
        } else ...
      

      I think that SqlTypeFactoryImpl#createTypeWithNullability should be modified in the same way when handling a BasicSqlType, to avoid calling BasicSqlType#createWithNullability (which internally executes a clone) if nullable matches:

      public RelDataType createTypeWithNullability(final RelDataType type, final boolean nullable) {
        RelDataType newType;
        if (type instanceof BasicSqlType) {
          if (type.isNullable() == nullable) { // new piece of code
            newType = type;
          } else { // previously, this block was always executed
            BasicSqlType sqlType = (BasicSqlType) type;
            newType = sqlType.createWithNullability(nullable);
          }
        } else ...
      

      I have verified, and Calcite Core unit tests run fine with this modification.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                julianhyde Julian Hyde
                Reporter:
                rubenql Ruben Q L
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: