Skip to content
This repository has been archived by the owner on Jun 14, 2022. It is now read-only.

Commit

Permalink
Remove unneeded strict-mode related methods
Browse files Browse the repository at this point in the history
This commit removes 3 exported methods relating to strict-mode detection:
* Instance.StrictModeCompliant
* Flavor.HasInnoFileFormat
* Flavor.InnoRowFormatReqs

Skeema v1.5.0 no longer enforces, requires, or assumes use of strict mode, so
these methods no longer have a caller, and there are no known third-party
open source callers of these methods. They're rather niche, and
Instance.StrictModeCompliant wasn't sufficiently thorough to fully solve its
intended use anyway. Additionally, now that MySQL 5.6 has reached its support
EOL date by Oracle, strict mode is enabled by default in all non-EOL versions
of MySQL and MariaDB.

This is a backwards-incompatible change; this package is still pre-1.0.
Even so, removal of exported methods is typically avoided, but in this case
the benefits of complexity reduction and faster test execution outweigh
keeping around the dead weight of unused code.
  • Loading branch information
evanelias committed Feb 24, 2021
1 parent 927c880 commit c932fb4
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 281 deletions.
34 changes: 0 additions & 34 deletions flavor.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,47 +300,13 @@ func (fl Flavor) AlwaysShowTableCollation(charSet string) bool {
return false
}

// HasInnoFileFormat returns true if the innodb_file_format variable exists in
// the flavor, false otherwise.
func (fl Flavor) HasInnoFileFormat() bool {
return !(fl.MySQLishMinVersion(8, 0) || fl.VendorMinVersion(VendorMariaDB, 10, 3))
}

// GeneratedColumns returns true if the flavor supports generated columns
// using MySQL's native syntax. (Although MariaDB 10.1 has support for generated
// columns, its syntax is borrowed from other DBMS, so false is returned.)
func (fl Flavor) GeneratedColumns() bool {
return fl.MySQLishMinVersion(5, 7) || fl.VendorMinVersion(VendorMariaDB, 10, 2)
}

// InnoRowFormatReqs returns information on the flavor's requirements for
// using the supplied row_format in InnoDB. If the first return value is true,
// the flavor requires innodb_file_per_table=1. If the second return value is
// true, the flavor requires innodb_file_format=Barracuda.
// The format arg must be one of "DYNAMIC", "COMPRESSED", "COMPACT", or
// "REDUNDANT" (case-insensitive), otherwise this method panics...
func (fl Flavor) InnoRowFormatReqs(format string) (filePerTable, barracudaFormat bool) {
switch strings.ToUpper(format) {
case "DYNAMIC":
// DYNAMIC is always OK in MySQL/Percona 5.7+, and MariaDB 10.1 or 10.3+.
// Oddly, MariaDB 10.2 is more picky and requires Barracuda.
if fl.MySQLishMinVersion(5, 7) {
return false, false
} else if fl.VendorMinVersion(VendorMariaDB, 10, 1) {
return false, (fl.Major == 10 && fl.Minor == 2)
}
return true, true
case "COMPRESSED":
// COMPRESSED always requires file_per_table, and it requires Barracuda in
// any flavor that still has the innodb_file_format variable.
return true, fl.HasInnoFileFormat()
case "COMPACT", "REDUNDANT":
return false, false
}
// Panic on unexpected input, since this may be programmer error / a typo
panic(fmt.Errorf("Unknown row_format %s is not supported", format))
}

// SortedForeignKeys returns true if the flavor sorts foreign keys
// lexicographically in SHOW CREATE TABLE.
func (fl Flavor) SortedForeignKeys() bool {
Expand Down
139 changes: 0 additions & 139 deletions flavor_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package tengo

import (
"fmt"
"testing"
)

Expand Down Expand Up @@ -240,91 +239,6 @@ func TestFlavorAlwaysShowTableCollation(t *testing.T) {

}

func TestFlavorHasInnoFileFormat(t *testing.T) {
type testcase struct {
receiver Flavor
expected bool
}
cases := []testcase{
{FlavorMySQL55, true},
{FlavorMySQL57, true},
{FlavorMySQL80, false},
{FlavorMariaDB102, true},
{FlavorMariaDB103, false},
{FlavorPercona57, true},
{FlavorPercona80, false},
{FlavorUnknown, true},
}
for _, tc := range cases {
actual := tc.receiver.HasInnoFileFormat()
if actual != tc.expected {
t.Errorf("Expected %s.HasInnoFileFormat() to return %t, instead found %t", tc.receiver, tc.expected, actual)
}
}
}

func (s TengoIntegrationSuite) TestFlavorHasInnoFileFormat(t *testing.T) {
flavor := s.d.Flavor()
db, err := s.d.Connect("", "")
if err != nil {
t.Fatalf("Unexpected error from Connect: %s", err)
}
var innoFileFormat string
err = db.QueryRow("SELECT @@global.innodb_file_format").Scan(&innoFileFormat)
expected := flavor.HasInnoFileFormat()
actual := (err == nil)
if expected != actual {
t.Errorf("Flavor %s expected existence of innodb_file_format is %t, instead found %t", flavor, expected, actual)
}
}

func TestInnoRowFormatReqs(t *testing.T) {
type testcase struct {
receiver Flavor
format string
expectFilePerTable bool
expectBarracuda bool
}
cases := []testcase{
{FlavorMySQL55, "DYNAMIC", true, true},
{FlavorMySQL56, "DYNAMIC", true, true},
{FlavorMySQL57, "DYNAMIC", false, false},
{FlavorMySQL57, "COMPRESSED", true, true},
{FlavorMySQL57, "COMPACT", false, false},
{FlavorMySQL80, "DYNAMIC", false, false},
{FlavorMySQL80, "COMPRESSED", true, false},
{FlavorPercona56, "COMPRESSED", true, true},
{FlavorPercona57, "DYNAMIC", false, false},
{FlavorMariaDB101, "DYNAMIC", false, false},
{FlavorMariaDB101, "REDUNDANT", false, false},
{FlavorMariaDB102, "DYNAMIC", false, true},
{FlavorMariaDB102, "COMPRESSED", true, true},
{FlavorMariaDB103, "DYNAMIC", false, false},
{FlavorMariaDB103, "COMPRESSED", true, false},
{NewFlavor("mariadb:5.5"), "DYNAMIC", true, true},
{FlavorUnknown, "DYNAMIC", true, true},
{FlavorUnknown, "COMPRESSED", true, true},
{FlavorUnknown, "COMPACT", false, false},
}
for _, tc := range cases {
actualFilePerTable, actualBarracuda := tc.receiver.InnoRowFormatReqs(tc.format)
if actualFilePerTable != tc.expectFilePerTable || actualBarracuda != tc.expectBarracuda {
t.Errorf("Expected %s.InnoRowFormatReqs(%s) to return %t,%t; instead found %t,%t", tc.receiver, tc.format, tc.expectFilePerTable, tc.expectBarracuda, actualFilePerTable, actualBarracuda)
}
}

var didPanic bool
defer func() {
if recover() != nil {
didPanic = true
}
}()
FlavorMySQL80.InnoRowFormatReqs("SUPER-DUPER-FORMAT")
if !didPanic {
t.Errorf("Expected InnoRowFormatReqs to panic on invalid format, but it did not")
}
}

func TestFlavorGeneratedColumns(t *testing.T) {
type testcase struct {
receiver Flavor
Expand Down Expand Up @@ -375,59 +289,6 @@ func TestSortedForeignKeys(t *testing.T) {
}
}

func (s TengoIntegrationSuite) TestInnoRowFormatReqs(t *testing.T) {
// Connect using innodb_strict_mode, which causes CREATE TABLE to fail if the
// ROW_FORMAT clause isn't allowed with current settings
db, err := s.d.Connect("testing", "innodb_strict_mode=1")
if err != nil {
t.Fatalf("Unexpected error from connect: %s", err)
}

exec := func(statement string) {
t.Helper()
if _, err := db.Exec(statement); err != nil {
t.Fatalf("Unexpected error from Exec: %s", err)
}
}
assertCanExec := func(expected bool, rowFormat string) {
t.Helper()
_, err := db.Exec(fmt.Sprintf("CREATE TABLE reqtest (id int unsigned) ROW_FORMAT=%s", rowFormat))
result := err == nil
if result != expected {
t.Errorf("assertCanExec failed: Expected %t, found %t", expected, result)
}
if result {
if _, err = db.Exec("DROP TABLE reqtest"); err != nil {
t.Fatalf("Unexpected error from Exec: %s", err)
}
}
}

// Confirm the flavor's actual requirements match InnoRowFormatReqs:
// Try creating table with each format under combinations of
// innodb_file_per_table and innodb_file_format, and confirm ability to create
// the table (under strict mode) matches expectation from return value of
// InnoRowFormatReqs.
for _, format := range []string{"DYNAMIC", "COMPRESSED"} {
needFPT, needBarracuda := s.d.Flavor().InnoRowFormatReqs(format)

exec("SET GLOBAL innodb_file_per_table=0")
db.Exec("SET GLOBAL innodb_file_format=Antelope") // ignore errors, var may not exist
assertCanExec(!needFPT && !needBarracuda, format)

exec("SET GLOBAL innodb_file_per_table=1")
assertCanExec(!needBarracuda, format)

exec("SET GLOBAL innodb_file_per_table=0")
db.Exec("SET GLOBAL innodb_file_format=Barracuda") // ignore errors, var may not exist
assertCanExec(!needFPT, format)
}

// Clean up globals
exec("SET GLOBAL innodb_file_per_table=DEFAULT")
db.Exec("SET GLOBAL innodb_file_format=DEFAULT") // ignore errors, var may not exist
}

func TestOmitIntDisplayWidth(t *testing.T) {
type testcase struct {
receiver Flavor
Expand Down
54 changes: 0 additions & 54 deletions instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -895,57 +895,3 @@ func (instance *Instance) DefaultCharSetAndCollation() (serverCharSet, serverCol
err = db.QueryRow("SELECT @@global.character_set_server, @@global.collation_server").Scan(&serverCharSet, &serverCollation)
return
}

// StrictModeCompliant returns true if all tables in the supplied schemas,
// if re-created on instance, would comply with innodb_strict_mode and a
// sql_mode including STRICT_TRANS_TABLES,NO_ZERO_DATE.
// This method does not currently detect invalid-but-nonzero dates in default
// values, although it may in the future.
func (instance *Instance) StrictModeCompliant(schemas []*Schema) (bool, error) {
var hasFilePerTable, hasBarracuda, alreadyPopulated bool
getFormatVars := func() (fpt, barracuda bool, err error) {
if alreadyPopulated {
return hasFilePerTable, hasBarracuda, nil
}
db, err := instance.CachedConnectionPool("", "")
if err != nil {
return false, false, err
}
var ifpt, iff string
if instance.Flavor().HasInnoFileFormat() {
err = db.QueryRow("SELECT @@global.innodb_file_per_table, @@global.innodb_file_format").Scan(&ifpt, &iff)
hasBarracuda = (strings.ToLower(iff) == "barracuda")
} else {
err = db.QueryRow("SELECT @@global.innodb_file_per_table").Scan(&ifpt)
hasBarracuda = true
}
hasFilePerTable = (ifpt == "1")
alreadyPopulated = (err == nil)
return hasFilePerTable, hasBarracuda, err
}

for _, s := range schemas {
for _, t := range s.Tables {
for _, c := range t.Columns {
if strings.HasPrefix(c.TypeInDB, "timestamp") || strings.HasPrefix(c.TypeInDB, "date") {
if strings.HasPrefix(c.Default, "'0000-00-00") {
return false, nil
}
}
}
if format := t.RowFormatClause(); format != "" {
needFilePerTable, needBarracuda := instance.Flavor().InnoRowFormatReqs(format)
if needFilePerTable || needBarracuda {
haveFilePerTable, haveBarracuda, err := getFormatVars()
if err != nil {
return false, err
}
if (needFilePerTable && !haveFilePerTable) || (needBarracuda && !haveBarracuda) {
return false, nil
}
}
}
}
}
return true, nil
}
54 changes: 0 additions & 54 deletions instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,60 +694,6 @@ func (s TengoIntegrationSuite) TestInstanceAlterSchema(t *testing.T) {
assertNoError("testing", "utf8mb4", "utf8mb4_general_ci", "utf8mb4", "utf8mb4_general_ci")
}

func (s TengoIntegrationSuite) TestInstanceStrictModeCompliant(t *testing.T) {
assertCompliance := func(expected bool) {
t.Helper()
schemas, err := s.d.Schemas()
if err != nil {
t.Fatalf("Unexpected error from Schemas: %s", err)
}
compliant, err := s.d.StrictModeCompliant(schemas)
if err != nil {
t.Fatalf("Unexpected error from StrictModeCompliant: %s", err)
}
if compliant != expected {
t.Errorf("Unexpected result from StrictModeCompliant: found %t", compliant)
}
}
db, err := s.d.Connect("testing", "innodb_strict_mode=0&sql_mode=%27NO_ENGINE_SUBSTITUTION%27")
if err != nil {
t.Fatalf("Unexpected error from connect: %s", err)
}
exec := func(statement string) {
t.Helper()
if _, err := db.Exec(statement); err != nil {
t.Fatalf("Unexpected error from Exec: %s", err)
}
}

// Default setup in integration.sql is expected to be compliant, except in 5.5
// due to use of zero default dates there only
major, minor, _ := s.d.Version()
expect := (major != 5 || minor != 5)
assertCompliance(expect)

if expect {
// A table with a zero-date default should break compliance
exec("CREATE TABLE has_zero_date (day date NOT NULL DEFAULT '0000-00-00')")
assertCompliance(false)
exec("DROP TABLE has_zero_date")
} else {
// 5.5 should become compliant if we drop the table with a zero-date default
exec("DROP TABLE grab_bag")
assertCompliance(true)
}

// Create tables with ROW_FORMAT=COMPRESSED. This should break compliance in
// MySQL/Percona 5.5-5.6 and MariaDB 10.1, due to their default globals.
exec("CREATE TABLE comprtest1 (name varchar(30)) ROW_FORMAT=COMPRESSED")
exec("CREATE TABLE comprtest2 (name varchar(30)) ROW_FORMAT=COMPRESSED")
expect = true
if (major == 5 && minor <= 6) || s.d.Flavor().Family() == FlavorMariaDB101 {
expect = false
}
assertCompliance(expect)
}

// TestColumnCompression confirms that various logic around compressed columns
// in Percona Server and MariaDB work properly. The syntax and functionality
// differs between these two vendors, and meanwhile MySQL has no equivalent
Expand Down

0 comments on commit c932fb4

Please sign in to comment.