Remove alt buffer handling

Author: malone-at-work

sql/field.h

@@ -1889,6 +1889,7 @@ class Field {
   const uchar *unpack_int64(uchar *to, const uchar *from) const;
 
  private:
+  // TODO(villagesql): Collapse these into one object (here and in Field)
   const villagesql::TypeContext *custom_type{nullptr};
   villagesql::TypeEncoder *type_encoder_{nullptr};
 

villagesql/types/type_encoder.cc

@@ -79,48 +79,35 @@ String *TypeEncoder::encode(const String &from, bool &is_valid) {
     vdf_->vdf(&ctx_, &vdf_args_, &vdf_result_);
 
     if (vdf_result_.type != VEF_RESULT_VALUE) {
-      // TODO(villagesql-beta): handle errors
+      // TODO(villagesql-beta): log warnings for errors
       is_valid = false;
       return nullptr;
     }
 
-    const size_t actual_len = vdf_result_.actual_len;
-
     if (alt_bin_buf_ != nullptr) {
-      // VDF used its own buffer (output exceeded buffer_size_). Grow
-      // overflow_buf_ if needed and reuse it across rows.
-      if (actual_len > overflow_buf_size_) {
-        auto *new_buf = new (mem_root_) uchar[actual_len];
-        if (should_assert_if_null(new_buf)) {
-          my_error(ER_OUTOFMEMORY, MYF(ME_FATALERROR), actual_len);
-          return nullptr;
-        }
-        overflow_buf_ = new_buf;
-        overflow_buf_size_ = actual_len;
-      }
-      if (actual_len > 0) memcpy(overflow_buf_, alt_bin_buf_, actual_len);
-      overflow_result_.set(pointer_cast<const char *>(overflow_buf_),
-                           actual_len, &my_charset_bin);
-      return &overflow_result_;
+      // TODO(villagesql-beta): support the extension returning alternate buffers
+      is_valid = false;
+      return nullptr;
     }
 
     // TODO(villagesql-beta): report an error or warning when the VDF overruns
-    // the buffer rather than silently returning invalid.
-    if (should_assert_if_false(actual_len <= buffer_size_)) {
+    // the buffer rather than silently returning invalid
+    const size_t actual_len = vdf_result_.actual_len;
+    if (actual_len > buffer_size_) {
       is_valid = false;
       return nullptr;
     }
+
     result_.set(buffer_, actual_len, &my_charset_bin);
   } else {
+    // TODO(villagesql-beta): Remove suport for these raw functions
     assert(fn_ != nullptr);
     size_t actual_length = 0;
     if (fn_(pointer_cast<uchar *>(buffer_), buffer_size_, from.ptr(),
             from.length(), &actual_length)) {
       is_valid = false;
       return nullptr;
     }
-    // TODO(villagesql-beta): report an error or warning when the fn_ overruns
-    // the buffer rather than silently returning invalid.
     if (should_assert_if_false(actual_length <= buffer_size_)) {
       is_valid = false;
       return nullptr;

villagesql/types/util.cc

@@ -245,14 +245,11 @@ bool HandleCustomColumnsForTableRename(THD &thd, const char *old_db,
 // the Field clone that caches the encoder pointer, so both are freed together
 // when the TABLE is evicted from the table open cache.
 //
-// For any kind of tmp table we use TABLE_SHARE::mem_root. TABLE::mem_root is
-// asserted empty by close_tmp_table() so it cannot be used. thd->mem_root
-// (the statement arena) is also wrong: the first TABLE's Fields happen to live
-// there for INTERNAL_TMP_TABLE, but additional TABLE instances sharing the
-// same TABLE_SHARE (opened via open_table_from_share()) have their cloned
-// Fields in share->mem_root, which outlives the statement. TABLE_SHARE::mem_root
-// therefore covers all cases and is freed by free_tmp_table() when the table
-// is torn down.
+// For any kind of tmp table use TABLE_SHARE::mem_root, this is where the
+// Fields are allocated for most uses of tmp tables. The one exception is
+// INTERNAL_TMP_TABLE where the initial Table object's fields are allocated
+// from the THD::mem_root, but for these tables the share is destroyed at the
+// end of the statement, so the lifetime is correct.
 static TypeEncoder *GetTypeEncoderFor(Field *field) {
   TypeEncoder *encoder = field->get_type_encoder();
   if (encoder == nullptr) {