maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Sergei,

sorry, I forgot to add that implementation with @@force_fields_visible
is cleaner and shorter. Also that helps to deprecate sysvers_show and
MDEV-16587 which I would greatly appreciate.

On Tue, Apr 27, 2021 at 4:07 PM Aleksey Midenkov <midenok@xxxxxxxxx> wrote:
>
> Hi Sergei!
>
> On Tue, Apr 6, 2021 at 3:29 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> >
> > Hi, Aleksey!
> >
> > On Apr 06, Aleksey Midenkov wrote:
> > > > > 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:
> > > >
> > > > I'm sorry, I don't understand.
> > > >
> > > > First, visibility is pretty much unrelated concept.
> > > > row_start/row_end can be visible or invisible, and they can be
> > > > writable or not writable - those are orthogonal concepts.
> > >
> > > To be able to specify them in INSERT command they must be at least
> > > user-invisible. System-invisible fields are ignored.
> >
> > Sure, that's what @@system_versioning_insert_history would do.
> >
>
> That's not about permission of inserting history which can be
> controlled by @@secure_timestamp setting. But rather that's about
> allowing to put system-invisible fields into INSERT command. You can
> suggest a better name for it.
>
> > > > And second, the name is wrong, there are no "fields" row_start and
> > > > row_end unless the user creates then explicitly. They are pieces of
> > > > metadata that every row has, something that Oracle, for example,
> > > > calls "pseudocolumns". Something like
> > > > @@system_versioning_row_start_row_end_visible would be more correct,
> > > > but ugly. In fact, I'd say that @@system_versioning_insert_history
> > > > was the best one.
> > >
> > > I think you are complicating things where complication is not needed.
> > > Pseudo- or not they are fields.
> >
> > No, this is internal implementation detail that can change and it should
> > not leak into the UI. Like, sql_select.cc has a function called
> > sub_select(), but we never tell users that what it does is "subselect",
> > not in the documentation, not in error messages, never. Because it
> > doesn't (it performs one step in a nested-loop join).
>
> Every concept should have a proper application. I don't think "double
> design" in hiding system versioning fields adds any value. I can not
> be sure about "subselect" design, but I suspect it will never be
> replaced by anything else. In any case each and every solution should
> be judged by whether it adds usability (i.e. ease of use) or not. I
> have a suspicion that the current design of hidden row_start/row_end
> adds more obstacles to a user than helps him.
>
> >
> > The fact that some internal enum value is named INVISIBLE_SYSTEM
> > is not something that should affect the user visible behavior.
> >
> > > Besides, the variable is not about system versioning. It can make any
> > > system-invisible fields visible.
> >
> > Which is incorrect. ROWNUM (MDEV-24089) is a pseudocolumn, and it cannot
> > be "visible" in the sense of @force_fields_visible. We don't support
> > ROWID, but if we would — it cannot be visible in the sense of
> > @force_fields_visible either. Basically this variable can only apply to
> > row_start/row_end pseudocolumns, despite its generic name.
>
> That is just the subject for a new feature request, isn't it? In any
> case, please suggest a different name. I can think of
> @@system_versioning_show_row_timestamps but this looks a bit long to
> me.
>
> >
> > Regards,
> > Sergei
> > VP of MariaDB Server Engineering
> > and security@xxxxxxxxxxx
>
>
>
> --
> All the best,
>
> Aleksey Midenkov
> @midenok



-- 
All the best,

Aleksey Midenkov
@midenok