Uploaded image for project: 'Kudu'
  1. Kudu
  2. KUDU-2280

Altering the column default isn't "type safe"

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Open
    • Critical
    • Resolution: Unresolved
    • 1.4.0
    • None
    • client

    Description

      When creating a table the schema data is used to check that a default value is of the right size and type. This is possible because the column schema is available and checked via KuduValue::Data->CheckTypeAndGetPointer  in KuduColumnSpec::ToColumnSchema.

      When altering a table KuduValue::Data->GetSlice() is used instead because we don't have the column schema information available. The Slice is then added to the alter table request. 

      When this request is received server side, we can only check the size (if we know the expected size) to validate the correct information was sent and cast it to the correct value via ColumnSchema::ApplyDelta.

      For some examples I can set a DOUBLE type default on an INT64 column, or a FLOAT type default on an INT32 column. With the current size check logic ( col_delta.default_value->size() < type_info()->size() ) you can technically set any type >= your target type. 

      An additional issue is that KuduValue::FromInt treats all integers as int64_t values when calling KuduValue::Data->GetSlice(). This means if we made the size check more strict we wouldn't be able to alter the default value of any integer columns smaller than INT64 because the data size is too large once received by the server. This "size" problem affects the ability to support decimal defaults too. 

      example (where column "default" is an INT32 column): 

      table_alterer->AlterColumn("default")->Default(KuduValue::FromInt(12345));

      To solve this we could require the expected column DataType to be passed along with the request so that the server can validate the expected column, size, and coerce the values smaller if needed/possible. 

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              granthenke Grant Henke
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated: