sqitchers/sqitch

Checkit function creation causing error when binary logging enabled - MySQL DB

Edwards80 opened this issue · 4 comments

We have binary logging enabled on a MySQL 5.7 DB but the deploy scripts fail with SQL Error 1419

"You do not have super user privilege and binary logging is enabled"

This error is caused by the creation of the checkit function, however we do not use the function in our verify scripts.

Is there a fix for this that doesn't involve setting log_bin_trust_function_creators to true?

I don't know, I would have to defer to a MySQL expert to determine the appropriate solution here.

@Edwards80 I ran into the same problem and I think the issue is that with binary logging enabled, CREATE FUNCTION is only allowed if both of the following conditions are met:

  • You have the SUPER privilege
  • The function is declared as DETERMINISTIC

As the checkit function is declared DETERMINISTIC, I assume that you do not have SUPER privilege. This is common for databases in the cloud (AWS, Azure, GCP). I do share your concerns regarding setting log_bin_trust_function_creators=1 as this makes replication unsafe.

@theory It would be great if sqitch could be changed to only issue a warning, when the creation of checkit failed. This could be achieved by putting the function creation in a separate script, catching the error when it is executed and logging a warning. I think most sane developers would prefer to live without checkit than to enable a setting that may break your database.

I think this issue affects all MySQL cloud databases that have a replica configured.

As I wanted to fix this problem for me, I dug a bit deeper into this. I was able to fix this locally for me, be removing the CREATE FUNCTION checkit and adding a procedure with the same name as a replacement.

App/Sqitch/Engine/mysql.sql:

CREATE
    PROCEDURE checkit(IN val int, IN message varchar(255))
BEGIN
    IF val != 1 THEN
        SIGNAL SQLSTATE '45000'
        SET MESSAGE_TEXT = message;
    END IF;
END;

AFAIK, the triggers do not have to be changed. The CREATE TRIGGER statements now referencing the procedure were accepted when I did a deployment. However, I have not really tested the triggers because my project does not use the dependency feature.

I think the best would be to patch the mysql.sql depending on the configuration of the MySQL server. If the user has no SUPER priveleges, @@log_bin = 1 and @@log_bin_trust_function_creators = 0, the procedure should be used instead of the function. A warning should be issued to inform the user that checkit exists as a procedure and therefore the usage in verify scripts is a bit different.

The following SQL can be used to fetch the relevant configuration data:

-- Check if we are allowed to create functions
SELECT  @@log_bin, @@log_bin_trust_function_creators,
    IF(SUM(privilege_type = 'SUPER') > 0, 'YES', 'NO') AS has_super
FROM
    information_schema.user_privileges
WHERE
    grantee = CONCAT("'", CURRENT_USER(), "'");

It would be great if someone with Perl knowledge could implement this, but if that's not happening, I may give it a try myself.

That sounds like a decent plan, and the mysql tests will ensure it functions properly on all (most?) supported versions of MySQL and MariaDB.

You can borrow from pg.pm's _run_registry_file method for how to check version numbers and use the command-line methods to detect various things and then modify the registry SQL script as appropriate. You'd want to make the changes you propose in the context of mysql.pm's run_upgrade method. I would assume that this change only needs to be applied as of some recent-ish version of MySQL, yes? Also MariaDB or no? Something like this:

if ($self->_fractional_seconds) {
    if ( $self->dbh->{mysql_serverversion} >= $someversion && $self->_probe('-c', q{
        SELECT  @@log_bin, @@log_bin_trust_function_creators,
                IF(SUM(privilege_type = 'SUPER') > 0, 'YES', 'NO') AS has_super
        FROM  information_schema.user_privileges
        WHERE  grantee = CONCAT("'", CURRENT_USER(), "'");
    }) {
        my $sql = scalar $file->slurp;
        # Replace checkit here.

        require File::Temp;
        my $fh = File::Temp->new;
        print $fh $sql;
        close $fh;
        $self->sqitch->run( @cmd, $self->_source($fh->filename) )
    } else {
        # Use previous code.
        $self->sqitch->run( @cmd, $self->_source($file) );
    }
}

Although it looks like that query returns "YES" or "NO"; better if it returns 1 or 0 in this context.