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@gmail.com> wrote:
Hi Sergei!
On Tue, Apr 6, 2021 at 3:29 PM Sergei Golubchik <serg@mariadb.org> 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@mariadb.org
-- All the best,
Aleksey Midenkov @midenok
-- All the best, Aleksey Midenkov @midenok