Hi!!
On Tue, Apr 1, 2025 at 9:47 PM Sergei Golubchik <[email protected]> wrote:
<cut>
> > MDEV-36425 Extend read_only to also block share locks and super user
<cut>
> > diff --git a/mysql-test/main/read_only.result
> > b/mysql-test/main/read_only.result
> > index 65cc12ffce9..89390ad65d5 100644
> > --- a/mysql-test/main/read_only.result
> > +++ b/mysql-test/main/read_only.result
> > @@ -69,11 +69,14 @@ set global read_only=1;
> > connection con1;
> > select @@global.read_only;
> > @@global.read_only
> > -0
> > +OFF
> > unlock tables ;
> > +Timeout in wait_condition.inc for SELECT @@global.read_only= 1
> > +Id User Host db Command Time State Info Progress
> > +5 test localhost test Query 0 starting show
> > full processlist 0.000
>
> Forgot to fix? It should wait for @@global.read_only='READ_ONLY' now
Yes, I had missed that.
I somehow thought that comparing the value with 1 would work, similar
to one allowing to set the value to 1.
Fixed.
> > --- a/sql/lock.cc
> > +++ b/sql/lock.cc
> > @@ -179,19 +198,48 @@ lock_tables_check(THD *thd, TABLE **tables, uint
> > count, uint flags)
<cut>
> > - if (t->reginfo.lock_type >= TL_FIRST_WRITE &&
> > - !ignore_read_only && opt_readonly && !thd->slave_thread)
> > + switch (opt_readonly) // Impossible
>
> you have "Impossible" comment on a wrong line
fixed
> > {
> > - my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only");
> > - DBUG_RETURN(1);
> > + case READONLY_OFF:
> > + DBUG_ASSERT(0);
> > + break;
> > + case READONLY_ON: // Compatibility
> > + if (!(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY) &&
> > + t->reginfo.lock_type >= TL_FIRST_WRITE)
> > + {
> > + mariadb_error_read_only();
> > + DBUG_RETURN(1);
>
> I'd make mariadb_error_read_only() to return 1. Then you
> can write DBUG_RETURN(mariadb_error_read_only());
> here and also in all other places, check it out, it's always followed by
> return 1.
I thought about that but not sure if it makes any sense.
I do not like to have a function that only returns one value.
It makes more sense to have the function as void.
The end result from the compiler should be the same.
> > + }
> > + break;
> > + case READONLY_NO_LOCK:
> > + if (!(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY) &&
> > + (thd->lex->sql_command == SQLCOM_LOCK_TABLES ||
> > + t->reginfo.lock_type >= TL_FIRST_WRITE ||
> > + t->reginfo.lock_type == TL_READ_WITH_SHARED_LOCKS))
>
> may be t->reginfo.lock_type >= TL_READ_WITH_SHARED_LOCKS ?
No, it has to be TL_READ_WITH_SHARED_LOCKS as we have higher locks like
TL_READ_SKIP_LOCKED that should not be part of this.
I did not want to change order of lock enum's in this patch.
<cut>
> > index d65cec37154..34ee8391009 100644
> > --- a/sql/sql_class.h
> > +++ b/sql/sql_class.h
> > @@ -3448,9 +3452,12 @@ class THD: public THD_count, /* this must be first */
> > */
> > inline bool is_read_only_ctx()
>
> rename to, for example, check_read_only_with_errror()
good idea, will do.
> > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
> > index 0247afd91a6..545b4674da1 100644
> > --- a/sql/sys_vars.cc
> > +++ b/sql/sys_vars.cc
<cut>
> > Sys_readonly(
> > "read_only",
> > - "Make all non-temporary tables read-only, with the exception for "
> > - "replication (slave) threads and users with the 'READ ONLY ADMIN' "
> > - "privilege",
> > - GLOBAL_VAR(read_only), CMD_LINE(OPT_ARG), DEFAULT(FALSE),
> > - NO_MUTEX_GUARD, NOT_IN_BINLOG,
> > + "Do not allow changes to non-temporary tables. Options are:"
> > + "OFF changes allowed. "
> > + "READ_ONLY blocks changes for users without the 'READ ONLY ADMIN' "
>
> "Disallow changes for users without the READ ONLY ADMIN privilege"
>
> > + "privilege. "
> > + "READ_ONLY_NO_LOCK blocks in addition LOCK TABLES and "
> > + "SELECT IN SHARE MODE for not ADMIN users. "
>
> "Additionally disallows LOCK TABLES and SELECT IN SHARE MODE"
>
> > + "READ_ONLY_NO_LOCK_NO_ADMIN blocks in addition 'ADMIN' users. "
>
> "Disallows also for users with READ_ONLY ADMIN privilege"
Fixed.
The only thing left is the naming of the options. Will call you about that.
Regards,
Monty
_______________________________________________
developers mailing list -- [email protected]
To unsubscribe send an email to [email protected]
