Hi, Nikita,

Looks good. One comment below:

On Aug 04, Nikita Malyavin wrote:
> revision-id: b508107c7a3 (mariadb-11.0.1-189-gb508107c7a3)
> parent(s): e9a75c98781
> author: Nikita Malyavin
> committer: Nikita Malyavin
> timestamp: 2023-07-29 00:13:48 +0400
> message:
>
> MDEV-31631 Adding auto-increment to table with history online misbehaves
>
> diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> index eabee13e8d3..3a70ed672a6 100644
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -9953,6 +9953,37 @@ const char *online_alter_check_supported(const THD *thd,
>      }
>    }
> 
> +  for (auto &c: alter_info->create_list)
> +  {
> +    *online= c.field || !(c.flags & AUTO_INCREMENT_FLAG);
> +    if (!*online)
> +      return "ADD COLUMN ... AUTO_INCREMENT";
> +
> +    auto *def= c.default_value;
> +    *online= !(def && def->flags & VCOL_NEXTVAL
> +            // either it's a new field, or a NULL -> NOT NULL change
> +             && (!c.field || (!(c.field->flags & NOT_NULL_FLAG)
> +                              && (c.flags & NOT_NULL_FLAG))));
> +    if (!*online)
> +    {
> +      const char *fmt= ER_THD(thd, ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED);
> +
> +      Item::vcol_func_processor_result res;
> +      IF_DBUG(bool ret=,)
> +      def->expr->walk(&Item::check_vcol_func_processor, 0, &res);

I don't think you need to walk here, res.name is always "nextval()"
isn't it?

> +      DBUG_ASSERT(!ret);
> +      DBUG_ASSERT(res.errors & VCOL_NEXTVAL);
> +
> +      LEX_CSTRING dflt{STRING_WITH_LEN("DEFAULT")};
> +      size_t len= strlen(fmt) + strlen(res.name) + c.field_name.length
> +                  + dflt.length;
> +      char *resp= (char*)thd->alloc(len);
> +      // expression %s cannot be used in the %s clause of %`s
> +      my_snprintf(resp, len, fmt, res.name, dflt.str, c.field_name.str);
> +      return resp;
> +    }
> +  }

Regards,
Sergei
VP of MariaDB Server Engineering
and security@mariadb.org