Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-29737

mysqldump doesn't check for --tz-utc when it accepts --dump-history

Details

    Description

      mysqldump refuses to run with --dump-history --compact saying

      bb-10.11-MDEV-16546 2b1d32429

      mysqldump: --dump-history requires 'SET time_zone' to work correctly
      

      However, it doesn't mind running when tz-utc itself is disabled, i.e. with --dump-history --skip-tz-utc, and producing a dump without SET TIME_ZONE.

      Attachments

        Issue Links

          Activity

            removed that requirement

            serg Sergei Golubchik added a comment - removed that requirement

            That's not really what I meant. I think the requirement makes sense, it just wasn't complete.
            Whatever (questionable) reasons people may have for disabling zero TZ in their usual dumps, and even if it can pass as "probably they know what they are doing", it hardly applies to history dump. Any mismatch in the TZ between the dumper and the loader will result in either the contents of versioned tables becoming unloadable (if row end gets converted into a value greater than max timestamp), or all current records becoming historical (if row end gets converted into a value smaller than max timestamp). The former is inconvenience, and it may take a while for a user to figure out what the TZ of the dumper was and fix it; the latter is in the essence data corruption.

            It's even worse if --compact is used. It's quite reasonable for users not to make a connection between the option and the contents of the historical data.
            For example, I didn't realize until recently that compact removes such a vital information from the dump, I think it's legacy-wrong in itself, regardless the history issue. It's one thing to remove comments and write less EOLs, and quite another to drop essential session settings without which the dump can easily become non-loadable (foreign keys, SQL mode etc.).

            elenst Elena Stepanova added a comment - That's not really what I meant. I think the requirement makes sense, it just wasn't complete. Whatever (questionable) reasons people may have for disabling zero TZ in their usual dumps, and even if it can pass as "probably they know what they are doing", it hardly applies to history dump. Any mismatch in the TZ between the dumper and the loader will result in either the contents of versioned tables becoming unloadable (if row end gets converted into a value greater than max timestamp), or all current records becoming historical (if row end gets converted into a value smaller than max timestamp). The former is inconvenience, and it may take a while for a user to figure out what the TZ of the dumper was and fix it; the latter is in the essence data corruption. It's even worse if --compact is used. It's quite reasonable for users not to make a connection between the option and the contents of the historical data. For example, I didn't realize until recently that compact removes such a vital information from the dump, I think it's legacy-wrong in itself, regardless the history issue. It's one thing to remove comments and write less EOLs, and quite another to drop essential session settings without which the dump can easily become non-loadable (foreign keys, SQL mode etc.).

            Yes, but that's what I really meant. I don't see how system versioning timestamp columns are different from any other timestamp columns. Any mismatch in the TZ between the dumper and the loader will result in the table being restored incorrectly this way or another.

            If we want to make mysqldump to reject --skip-tz-utc if any of the columns is timestamp — it'd be at least consistent. Or if we'd made --compact to not include --skip-tz-utc — that would make loading tables with timestamp columns a bit safer. But doing anything special for AS ROW START and AS ROW END columns as if timestamps there are any more fragile as other timestamps — this is questionable.

            serg Sergei Golubchik added a comment - Yes, but that's what I really meant. I don't see how system versioning timestamp columns are different from any other timestamp columns. Any mismatch in the TZ between the dumper and the loader will result in the table being restored incorrectly this way or another. If we want to make mysqldump to reject --skip-tz-utc if any of the columns is timestamp — it'd be at least consistent. Or if we'd made --compact to not include --skip-tz-utc — that would make loading tables with timestamp columns a bit safer. But doing anything special for AS ROW START and AS ROW END columns as if timestamps there are any more fragile as other timestamps — this is questionable.

            I won't insist on the change, but for the record, there is a considerable difference between system versioning timestamp columns and any other timestamp columns.
            All other columns are user-defined, people chose them to be timestamps. System versioning, at least in its simplest form with automatic row start/end, uses timestamp as internal implementation. Users can't see the columns' definition, and even if they know how to query the contents, they can only see a a value which can just as well be coming from a datetime or even a string column. All in all, I wouldn't claim they are obliged to keep in mind a connection between the timezone value in their dump (and even more so the compactness of the dump) and the health of the historical data it is dumping.

            elenst Elena Stepanova added a comment - I won't insist on the change, but for the record, there is a considerable difference between system versioning timestamp columns and any other timestamp columns. All other columns are user-defined, people chose them to be timestamps. System versioning, at least in its simplest form with automatic row start/end, uses timestamp as internal implementation. Users can't see the columns' definition, and even if they know how to query the contents, they can only see a a value which can just as well be coming from a datetime or even a string column. All in all, I wouldn't claim they are obliged to keep in mind a connection between the timezone value in their dump (and even more so the compactness of the dump) and the health of the historical data it is dumping.

            Alternatively, I can make time_zone='+00:00' to be written into the dump even if --compact. But then it'll beg the question, why time zone is written, while character set and all other settings that are necessary for the dump correctness, are not.

            I'd say that the easy rule is: if you want to make sure that the dump can be loaded correctly — don't use --compact

            serg Sergei Golubchik added a comment - Alternatively, I can make time_zone='+00:00' to be written into the dump even if --compact . But then it'll beg the question, why time zone is written, while character set and all other settings that are necessary for the dump correctness, are not. I'd say that the easy rule is: if you want to make sure that the dump can be loaded correctly — don't use --compact

            I'd say that the easy rule is: if you want to make sure that the dump can be loaded correctly — don't use --compact

            I agree with this, but then we should probably deprecate the option (and later remove or at least declare it dangerous and not recommended legacy), because it makes no sense to provide it knowing that as a rule it makes a dump to be further loaded incorrectly.
            Of course, it's out of the scope of this bug or the parent task.

            elenst Elena Stepanova added a comment - I'd say that the easy rule is: if you want to make sure that the dump can be loaded correctly — don't use --compact I agree with this, but then we should probably deprecate the option (and later remove or at least declare it dangerous and not recommended legacy), because it makes no sense to provide it knowing that as a rule it makes a dump to be further loaded incorrectly. Of course, it's out of the scope of this bug or the parent task.

            People

              serg Sergei Golubchik
              elenst Elena Stepanova
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.