Skip to content

Commit

Permalink
linter: add new option --lint-name-case
Browse files Browse the repository at this point in the history
This new linter rule checks for tables whose names contain uppercase
letters. Using uppercase letters in table names can be problematic when using
a mix of database server operating systems (e.g. Linux in prod but MacOS or
Windows in local dev) due to case-sensitive vs case-insensitive filesystem
behavior.

This rule also examines the casing used in the *.sql CREATE statement. On DB
servers using lower_case_table_names=1 (which forces table names to lower-
case), tables that had uppercase letters in their original CREATE statement
will still be flagged by this checker, even though LCTN=1 downcases them in
SHOW CREATE TABLE and information_schema.

This linter rule is disabled by default, since many companies avoid name-
casing problems by only using Linux on the database server side, or by
strictly following the lower_case_table_names advice in the MySQL manual:
https://dev.mysql.com/doc/refman/8.0/en/identifier-case-sensitivity.html
  • Loading branch information
evanelias committed May 25, 2022
1 parent 0d5d476 commit 8cb30d0
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 1 deletion.
52 changes: 52 additions & 0 deletions internal/linter/check_name_case.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package linter

import (
"fmt"
"strings"

"github.com/skeema/skeema/internal/tengo"
)

func init() {
RegisterRule(Rule{
CheckerFunc: TableBinaryChecker(nameCaseChecker),
Name: "name-case",
Description: "Flag tables that have uppercase letters in their names",
DefaultSeverity: SeverityIgnore,
})
}

func nameCaseChecker(table *tengo.Table, createStatement string, _ *tengo.Schema, _ Options) *Note {
var message string

// Simple comparison is reliable with lower_case_table_names=0 or 2. However,
// with lower_case_table_names=1, table.Name will already be forced lowercase
// by the database itself. So we need to confirm the name exists with the same
// casing in its original (non-canonicalized) CREATE statement as well.
if strings.ToLower(table.Name) == table.Name {
// Non-canonicalized CREATE may include arbitrary whitespace, and may or may
// not use backticks. We just want to check the CREATE segment after "table"
// and before the first open-paren, unless we can't find them (e.g. CREATE
// TABLE ... LIKE), in which case we fall back to searching the full CREATE.
var startPos, endPos int
if endPos = strings.Index(createStatement, "("); endPos < 0 {
endPos = len(createStatement)
}
if tableKeywordPos := strings.Index(strings.ToLower(createStatement[0:endPos]), "table"); tableKeywordPos >= 0 {
startPos = tableKeywordPos + 5 // len("table")
}
if strings.Contains(createStatement[startPos:endPos], table.Name) {
return nil
}
message = "Table name %s used uppercase letters in its original CREATE statement, but these were automatically down-cased by the database server's lower_case_table_names=1 setting. This can impact data portability if any of your environments use a different lower_case_table_names setting."
} else {
message = "Table name %s contains uppercase letters. This affects data portability if you use a mix of operating systems, e.g. Linux for production databases but MacOS or Windows for local development databases. Table names are case-sensitive in queries on Linux database servers, but not on Windows or MacOS."
}

message = fmt.Sprintf(message, tengo.EscapeIdentifier(table.Name)) + " To avoid name-casing portability issues, use only lowercase letters when naming new tables.\n(Do NOT adjust name-casing for existing tables, as this would break queries on Linux database servers! RENAME TABLE operations cannot be handled directly by Skeema.)"
return &Note{
LineOffset: 0,
Summary: "Table name contains uppercase letters",
Message: message,
}
}
5 changes: 5 additions & 0 deletions internal/linter/testdata/validcfg/namecase.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CREATE TABLE NameCase ( /* annotations:name-case */
id int unsigned NOT NULL,
name varchar(30),
PRIMARY KEY (id)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
16 changes: 15 additions & 1 deletion lctn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,19 @@ func (s SkeemaIntegrationSuite) TestLowerCaseTableNames1(t *testing.T) {
s.handleCommand(t, CodeSuccess, ".", "skeema init --dir mydb --schema NameCase -h %s -P %d lctn0", s.d.Instance.Host, s.d.Instance.Port)
s.handleCommand(t, CodeSuccess, ".", "skeema diff lctn0")

// Add an environment for the lctn=2 instance, and then push to it. Afterwards,
// Add an environment for the lctn=1 instance, and then push to it. Afterwards,
// diff should show no differences.
s.handleCommand(t, CodeSuccess, ".", "skeema add-environment --dir mydb -h %s -P %d lctn1", dinst.Instance.Host, dinst.Instance.Port)
s.handleCommand(t, CodeSuccess, ".", "skeema push lctn1")
s.handleCommand(t, CodeSuccess, ".", "skeema diff lctn1")

// lint should show no problems on either environment by default, unless
// lint-name-case is enabled
s.handleCommand(t, CodeSuccess, ".", "skeema lint lctn0 --skip-format")
s.handleCommand(t, CodeSuccess, ".", "skeema lint lctn1 --skip-format")
s.handleCommand(t, CodeFatalError, ".", "skeema lint lctn0 --skip-format --lint-name-case=error")
s.handleCommand(t, CodeFatalError, ".", "skeema lint lctn1 --skip-format --lint-name-case=error")

// Confirm all tables on the LCTN=1 db are supported for diff operations
schema, err := dinst.Schema("NameCase")
if err != nil {
Expand Down Expand Up @@ -182,6 +189,13 @@ func (s SkeemaIntegrationSuite) TestLowerCaseTableNames2(t *testing.T) {
s.handleCommand(t, CodeSuccess, ".", "skeema push lctn2")
s.handleCommand(t, CodeSuccess, ".", "skeema diff lctn2")

// lint should show no problems on either environment by default, unless
// lint-name-case is enabled
s.handleCommand(t, CodeSuccess, ".", "skeema lint lctn0 --skip-format")
s.handleCommand(t, CodeSuccess, ".", "skeema lint lctn2 --skip-format")
s.handleCommand(t, CodeFatalError, ".", "skeema lint lctn0 --skip-format --lint-name-case=error")
s.handleCommand(t, CodeFatalError, ".", "skeema lint lctn2 --skip-format --lint-name-case=error")

// Confirm all tables on the LCTN=2 db are supported for diff operations, and
// confirm the schema name comes back with original casing
schema, err := dinst.Schema("NameCase")
Expand Down

0 comments on commit 8cb30d0

Please sign in to comment.