This is intentional. mysql.user view is a compatibility feature for applications written for MariaDB-10.3 or earlier. As such, it's frozen and contains exactly the structure of mysql.user as it was in 10.3. It does not matter what new features MariaDB started to support since 10.3, it will never change the fact that password_expired was always 'N' in 10.3

Applications for 10.3 don't update when the server gets updated and could be assuming that because the field there is that its accurate. I'm not sure how a static value maps to compatibility when its not accurate or what anyone expects.

Sure we can wholesale change for compatibility like:
https://github.com/major/MySQLTuner-perl/pull/514/commits/0eef9260a8b37453a7787d5e63bccdda5d464380

Fine with frozen number of fields, but why leave a field as incorrect? Did we need to change this before 10.4 GA?

the original idea was that we'll never need to change the view definition, no mariadb-upgrade hassles at all.
but we've already broken that with adding mariadb.sys user — it required mariadb-upgrade to recreate the view.

I suppose, it doesn't matter much anymore. if the view is recreated anyway, we can keep doing it, I guess.

I've been thinking about this and I don't believe it is a good idea to add all those 4 columns in the user view all of a sudden. I think it is reasonable to add as a "feature" so to say in 10.6 (not sure it's a good idea to add as a bug fix in 10.4) the password_expired column to be in sync with the data in global_priv, because it was already there and it kind of makes sense for it to reflect the value from global_priv. But I wouldn't add the other 3, it sounds confusing to me and I can bet it will be confusing for other people in the future.

Let me know Daniel if you're ok with just making password_expired from user table be in sync with global_priv. Also can you please let me know how did this issue get in your way? I'm really curious to see what use case led you to this design detail.

Also to be clear, I'm open to counter-arguments to doing this differently, the paragraph above is based on what usually seems and doesn't seem confusing to me.

Per above I'm happy with a frozen number of fields.

Yes happy to keep this issue around password_expired being accurate.

The attention to this was prompted by a stackoverflow post of mysqltuner recommending setting a password on mariadb.sys user (which is obviously a horrible recommendation). Initial attempts to fix this in a MySQL/MariaDB compatible way looked at password_expire/account_locked to discover MariaDB was lying on one and missing on the other. MySQLtuner was fixed per above with a version dependent fix.

There are applications that are designed/tested on MySQL and expect MariaDB to be in sync. Version dependant fixes create a testing burden for projects and this practicality should be considered because MariaDB isn't considered the primary or supported database on a significant number of open source projects. Its easy to be dogmatic about adding columns/rows because some (Postel law breaking) application could break. Well written applications however will work fine and even medium quality ones that consider positional columns will also work with columns appended. We need to consider this against some well intentioned application needing the behaviour.

So for now, lets restrict this to password_expired, however a more comprehensive value assessment is needed if we do find other project(s) needing the other fields.

Thanks for the thorough comment Daniel, really liked it. Added a patch here: https://github.com/MariaDB/server/commit/7bf58bbbf411aa433d83c4f8e0092a5b119589d5
We will cherry-pick it into 10.4 once the tests are done and current release process is over.

ok. Patch look like a good start for the install case. An upgrade path however will probably need to drop and re-create the view by adding to the definition https://github.com/MariaDB/server/blob/10.4/scripts/mysql_system_tables_fix.sql#L814

e.g:

SELECT count(*) FROM information_schema.VIEWS WHERE TABLE_CATALOG = 'def' and TABLE_SCHEMA = 'mysql' and TABLE_NAME='user' and (DEFINER = 'root@localhost' OR LOCATE("'N' AS password_expired", VIEW_DEFINITION) IS NOT NULL)

Why would you need to re-create the view? For the upgrade path (you mean upgrade from mysql or upgrade from previous mariadb?), as far as I know we create mysql.global_priv from mysql.user table data, then drop mysql, then the view is created based on the data from mysql.global_priv, this sounds to me like the fix should work. Upgrade from previous mariadb should be similar.

upgrade from a mariadb-10.4+ instance before this is change committed. The view creation in scripts/mysql_system_tables_fix.sql is `create view if not exists` so an existing one doesn't get replaced. There is something odd about 0253ea7f2208 `DROP VIEW IF EXISTS` not being before the `create view if exists`.

After you compile the server, if you look into the generated file mysql_fix_privilege_tables_sql.c, the sql code from mysql_system_tables_fix.sql is the first being executed immediately followed by the code from mysql_system_tables.sql. It's a bit counter-intuitive if you only look at the names of these two files.
So the DROP VIEW IF EXISTS statement makes sense if the definer of the view is root@localhost (i.e. mariadb 10.4+ before mariadb.sys is introduced), because the view is dropped in the _fix.sql file, but then it is recreated in mysql_system_tables.sql with definer=mariadb.sys

But it seems to me that we need to drop the view unconditionally for every mysql_upgrade to make this patch work on current 10.4+ versions. I need to check with Vicentiu or Sergei and try to understand if there are any serious implications if we do this.

Or of course, drop the view conditionally based on parsing the value of the query view from SHOW CREATE VIEW, but this is very very ugly IMO.

> mysql_fix_privilege_tables_sql.c

Thanks for the explanation.

> But it seems to me that we need to drop the view unconditionally for every mysql_upgrade to make this patch work on current 10.4+ versions. I need to check with Vicentiu or Sergei

See the 3rd comment.

> Or of course, drop the view conditionally based on parsing the value of the query view from SHOW CREATE VIEW, but this is very very ugly IMO.

Its very very practical IMO. There is no ALTER VIEW, a view's existence is parsed. As a system VIEW its definition is rather fixed and providing you're are matching something fairly strict there should be a low change of failure. And the consequence of a mismatch - the view gets recreated.

Hey danblack, after struggling to understand how to fix some failing tests and how to add a new test for this mdev without listing again all the fields of the mysql.user view and add one more maintainance nightmare for the future, I think I managed to find a solution that would do.

Feel free to review it: https://github.com/MariaDB/server/commit/8fbf872b6e7d3c2d743619b796f55c94964d4850

Tests are green on both buildbots on bb-10.4-robert

Also I proposed something like this to Sanja to further reduce future pain, but let's wait till he gets back from vacation and see if he agrees. https://gist.github.com/robertbindar/63e24ee79eb99c60e961d5cbb026cc89

Pushed into 10.4. Followup fix to refactor MDEV-24363 tests also pushed into 10.4

thanks robertbindar