← Back to team overview

maria-developers team mailing list archive

Re: ef2519fee4e: MDEV-16546 System versioning setting to allow history modification

 

Sergei,

On Thu, Feb 25, 2021 at 11:09 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Feb 25, Aleksey Midenkov wrote:
> > > On Nov 24, Aleksey Midenkov wrote:
> > > > 1. Add server variable system_versioning_insert_history which will
> > > > allow INSERT history rows.
> > > > 2. If secure_timestamp is YES or REPLICATION,
> > > > system_versioning_insert_history does not have effect. If
> > > > secure_timestamp is SUPER, system_versioning_insert_history
> > > > requires special privilege (same as for setting current
> > > > timestamp).
> > >
> > > Yes. It is correct, but could be expressed simpler:
> > >
> > >   when @@system_versioning_insert_history=TRUE, one can directly insert
> > >   into row_start/row_end columns iff one can set @@timestamp variable.
> > >
> > > In other words, if one can create an arbitrary history by
> > > manipulating @@timestamp variable,
> > > @@system_versioning_insert_history allows to do it more
> > > conveniently. But if one cannot - he cannot.
> >
> > Then do we need additional setting @@system_versioning_insert_history?
> > Iff one can manipulate history via @@timestamp variable let him set
> > row_start/row_end columns.
>
> Sure, that's possible.
>
> I just thought @@system_versioning_insert_history could be like an extra
> safety (not security) measure. To prevent history from being modified
> unintentionally.
>

Well, unless one specified row_start/row_end explicitly he is safe.

But, since we need to specify implicit system fields we cannot avoid
adding one more session variable. In my current iteration I made
@@force_fields_visible which is more straightforward in what it does:

--- a/sql/table.cc
+++ b/sql/table.cc
@@ -4024,6 +4024,9 @@ enum open_frm_error open_table_from_share(THD
*thd, TABLE_SHARE *share,
  {
    if (!((*field_ptr)= share->field[i]->clone(&outparam->mem_root, outparam)))
      goto err;
+    if (thd->variables.force_fields_visible &&
+        (*field_ptr)->invisible <= INVISIBLE_SYSTEM)
+      (*field_ptr)->invisible= VISIBLE;
  }
  (*field_ptr)= 0;                              // End marker

This variable is more powerful as it affects any SQL command and I
hope this can be helpful. Please review the updated task in
bb-10.6-midenok

-- 
All the best,

Aleksey Midenkov
@midenok


Follow ups

References