Skip to content

Quote identifiers in SHOW CREATE PROCEDURE/FUNCTION for MySQL and MariaDB#299

Merged
tianzhou merged 1 commit intobytebase:mainfrom
lawrence3699:fix/quote-identifiers-show-create-procedure
Apr 3, 2026
Merged

Quote identifiers in SHOW CREATE PROCEDURE/FUNCTION for MySQL and MariaDB#299
tianzhou merged 1 commit intobytebase:mainfrom
lawrence3699:fix/quote-identifiers-show-create-procedure

Conversation

@lawrence3699
Copy link
Copy Markdown
Contributor

Ran into this while using search_objects with detail_level=full against a MySQL database that has a schema named with a reserved word. The SHOW CREATE PROCEDURE query failed with a syntax error because the schema and procedure names weren't being quoted.

Looking at the code, the MySQL and MariaDB connectors both interpolate schemaValue and procedureName directly into SHOW CREATE PROCEDURE and SHOW CREATE FUNCTION statements:

SHOW CREATE PROCEDURE ${schemaValue}.${procedureName}

Names containing spaces, reserved words (like order, select), dots, or backticks produce invalid SQL here. The rest of the codebase already handles this — the SQLite connector uses quoteIdentifier() for PRAGMA statements, and the search-objects tool uses quoteQualifiedIdentifier() for COUNT queries. These four call sites were the only ones missing it.

Before: SHOW CREATE PROCEDURE order.get_totals → MySQL syntax error
After: SHOW CREATE PROCEDURE \order`.`get_totals`` → works correctly

The fix adds quoteIdentifier() calls (from the existing identifier-quoter.ts utility) to the four affected SHOW CREATE statements in both mysql/index.ts and mariadb/index.ts.

Tests added: procedure-identifier-quoting.test.ts — 7 cases covering spaces, reserved words, backtick escaping, dots, and MariaDB parity.

…iaDB

Schema and procedure names containing spaces, reserved words, dots,
or backticks cause syntax errors in SHOW CREATE PROCEDURE/FUNCTION
statements because they were interpolated without quoting.

Use the existing quoteIdentifier() utility (already used by the SQLite
connector and search-objects tool) to properly backtick-quote these
identifiers, matching the pattern used throughout the codebase.
@lawrence3699 lawrence3699 requested a review from tianzhou as a code owner April 3, 2026 07:46
Copilot AI review requested due to automatic review settings April 3, 2026 07:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes MySQL/MariaDB search_objects(detail_level=full) failures when routine schema/procedure/function names require identifier quoting (e.g., reserved words, spaces), by ensuring SHOW CREATE PROCEDURE/FUNCTION uses backtick-quoted identifiers.

Changes:

  • Use quoteIdentifier() to quote schema and routine names when building SHOW CREATE PROCEDURE queries in the MySQL connector.
  • Use quoteIdentifier() to quote schema and routine names when building SHOW CREATE FUNCTION queries in the MySQL connector.
  • Apply the same quoting updates to the MariaDB connector and add a new test file around expected quoted SQL fragments.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/connectors/mysql/index.ts Quotes schema and routine identifiers in SHOW CREATE PROCEDURE/FUNCTION queries to handle reserved words/special chars.
src/connectors/mariadb/index.ts Mirrors the MySQL quoting fix for MariaDB SHOW CREATE queries.
src/connectors/tests/procedure-identifier-quoting.test.ts Adds tests asserting expected quoted SQL fragments for various identifier edge cases.

Comment on lines 476 to 486
const schemaValue = schema || (await this.getCurrentSchema());

// For full definition - different approaches based on type
const quotedSchema = quoteIdentifier(schemaValue, "mysql");
const quotedProcName = quoteIdentifier(procedureName, "mysql");
if (procedure.procedure_type === "procedure") {
// Try to get the definition from SHOW CREATE PROCEDURE
try {
const [defRows] = (await this.pool.query(`
SHOW CREATE PROCEDURE ${schemaValue}.${procedureName}
SHOW CREATE PROCEDURE ${quotedSchema}.${quotedProcName}
`)) as [any[], any];
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new quoting logic here isn’t exercised by tests: the added test file only asserts quoteIdentifier() output and string concatenation, so it would still pass even if this connector reverted to interpolating schemaValue/procedureName unquoted. Consider adding a unit test around getStoredProcedureDetail() that mocks pool.query() and asserts the issued SHOW CREATE ... SQL uses quoted identifiers for both schema and routine names.

Copilot uses AI. Check for mistakes.
Comment on lines 468 to 478
const schemaValue = schema || (await this.getCurrentSchema());

// For full definition - different approaches based on type
const quotedSchema = quoteIdentifier(schemaValue, "mariadb");
const quotedProcName = quoteIdentifier(procedureName, "mariadb");
if (procedure.procedure_type === "procedure") {
// Try to get the definition from SHOW CREATE PROCEDURE
try {
const defRows = await this.pool.query(`
SHOW CREATE PROCEDURE ${schemaValue}.${procedureName}
SHOW CREATE PROCEDURE ${quotedSchema}.${quotedProcName}
`) as any[];
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new quoting logic here isn’t exercised by tests: the added test file only asserts quoteIdentifier() output and string concatenation, so it would still pass even if this connector reverted to interpolating schemaValue/procedureName unquoted. Consider adding a unit test around getStoredProcedureDetail() that mocks pool.query() and asserts the issued SHOW CREATE ... SQL uses quoted identifiers for both schema and routine names.

Copilot uses AI. Check for mistakes.
Comment on lines +5 to +69
* Verify that MySQL/MariaDB SHOW CREATE PROCEDURE/FUNCTION statements
* use properly quoted identifiers. Procedure and schema names containing
* spaces, reserved words, or special characters cause syntax errors
* without backtick quoting.
*
* The connectors build SQL like:
* SHOW CREATE PROCEDURE ${quotedSchema}.${quotedProcName}
*
* This test validates the quoting produces valid SQL fragments.
*/
describe("SHOW CREATE PROCEDURE identifier quoting", () => {
describe("MySQL/MariaDB backtick quoting for procedure names", () => {
it("should quote names with spaces", () => {
const schema = quoteIdentifier("my schema", "mysql");
const proc = quoteIdentifier("my procedure", "mysql");
const sql = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE PROCEDURE `my schema`.`my procedure`");
});

it("should quote reserved words", () => {
const schema = quoteIdentifier("order", "mysql");
const proc = quoteIdentifier("select", "mysql");
const sql = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE PROCEDURE `order`.`select`");
});

it("should escape backticks in names", () => {
const schema = quoteIdentifier("db`name", "mysql");
const proc = quoteIdentifier("proc`name", "mysql");
const sql = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE PROCEDURE `db``name`.`proc``name`");
});

it("should quote names with dots", () => {
const schema = quoteIdentifier("my.db", "mysql");
const proc = quoteIdentifier("calc.totals", "mysql");
const sql = `SHOW CREATE FUNCTION ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE FUNCTION `my.db`.`calc.totals`");
});

it("should also work for MariaDB (same syntax)", () => {
const schema = quoteIdentifier("test schema", "mariadb");
const proc = quoteIdentifier("get-data", "mariadb");
const sql = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE PROCEDURE `test schema`.`get-data`");
});
});

describe("unquoted identifiers break with special characters", () => {
it("demonstrates the problem: spaces in names produce invalid SQL", () => {
const schema = "my schema";
const proc = "my procedure";
const unquotedSQL = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
// This SQL is invalid and will cause MySQL syntax errors
expect(unquotedSQL).toBe("SHOW CREATE PROCEDURE my schema.my procedure");
// The database would parse "my" as the schema and "schema.my" as noise
});

it("demonstrates the problem: reserved words produce invalid SQL", () => {
const schema = "order";
const proc = "select";
const unquotedSQL = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
// "order" and "select" are reserved words; MySQL will misparse this
expect(unquotedSQL).toBe("SHOW CREATE PROCEDURE order.select");
});
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test file largely re-tests quoteIdentifier() behavior (spaces, reserved words, backtick escaping, dots), which is already covered in src/utils/__tests__/identifier-quoter.test.ts. Since it doesn’t import or exercise the MySQL/MariaDB connectors, it won’t catch regressions in the actual SHOW CREATE ... call sites. Consider either (a) moving/merging these cases into the existing identifier-quoter tests, or (b) rewriting this to mock the connectors’ pool.query() and assert the generated SHOW CREATE SQL is quoted.

Suggested change
* Verify that MySQL/MariaDB SHOW CREATE PROCEDURE/FUNCTION statements
* use properly quoted identifiers. Procedure and schema names containing
* spaces, reserved words, or special characters cause syntax errors
* without backtick quoting.
*
* The connectors build SQL like:
* SHOW CREATE PROCEDURE ${quotedSchema}.${quotedProcName}
*
* This test validates the quoting produces valid SQL fragments.
*/
describe("SHOW CREATE PROCEDURE identifier quoting", () => {
describe("MySQL/MariaDB backtick quoting for procedure names", () => {
it("should quote names with spaces", () => {
const schema = quoteIdentifier("my schema", "mysql");
const proc = quoteIdentifier("my procedure", "mysql");
const sql = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE PROCEDURE `my schema`.`my procedure`");
});
it("should quote reserved words", () => {
const schema = quoteIdentifier("order", "mysql");
const proc = quoteIdentifier("select", "mysql");
const sql = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE PROCEDURE `order`.`select`");
});
it("should escape backticks in names", () => {
const schema = quoteIdentifier("db`name", "mysql");
const proc = quoteIdentifier("proc`name", "mysql");
const sql = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE PROCEDURE `db``name`.`proc``name`");
});
it("should quote names with dots", () => {
const schema = quoteIdentifier("my.db", "mysql");
const proc = quoteIdentifier("calc.totals", "mysql");
const sql = `SHOW CREATE FUNCTION ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE FUNCTION `my.db`.`calc.totals`");
});
it("should also work for MariaDB (same syntax)", () => {
const schema = quoteIdentifier("test schema", "mariadb");
const proc = quoteIdentifier("get-data", "mariadb");
const sql = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
expect(sql).toBe("SHOW CREATE PROCEDURE `test schema`.`get-data`");
});
});
describe("unquoted identifiers break with special characters", () => {
it("demonstrates the problem: spaces in names produce invalid SQL", () => {
const schema = "my schema";
const proc = "my procedure";
const unquotedSQL = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
// This SQL is invalid and will cause MySQL syntax errors
expect(unquotedSQL).toBe("SHOW CREATE PROCEDURE my schema.my procedure");
// The database would parse "my" as the schema and "schema.my" as noise
});
it("demonstrates the problem: reserved words produce invalid SQL", () => {
const schema = "order";
const proc = "select";
const unquotedSQL = `SHOW CREATE PROCEDURE ${schema}.${proc}`;
// "order" and "select" are reserved words; MySQL will misparse this
expect(unquotedSQL).toBe("SHOW CREATE PROCEDURE order.select");
});
* Keep this suite focused on SHOW CREATE statement construction.
*
* Detailed identifier escaping behavior is already covered in
* src/utils/__tests__/identifier-quoter.test.ts; these tests only verify
* that SHOW CREATE PROCEDURE/FUNCTION statements are assembled with quoted
* schema and routine identifiers.
*/
function buildShowCreateSQL(
dialect: "mysql" | "mariadb",
routineType: "PROCEDURE" | "FUNCTION",
schemaName: string,
routineName: string,
) {
const quotedSchema = quoteIdentifier(schemaName, dialect);
const quotedRoutine = quoteIdentifier(routineName, dialect);
return `SHOW CREATE ${routineType} ${quotedSchema}.${quotedRoutine}`;
}
describe("SHOW CREATE routine SQL assembly", () => {
it("builds a quoted MySQL SHOW CREATE PROCEDURE statement", () => {
const sql = buildShowCreateSQL(
"mysql",
"PROCEDURE",
"my schema",
"my procedure",
);
expect(sql).toBe("SHOW CREATE PROCEDURE `my schema`.`my procedure`");
});
it("builds a quoted MySQL SHOW CREATE FUNCTION statement", () => {
const sql = buildShowCreateSQL(
"mysql",
"FUNCTION",
"my.db",
"calc.totals",
);
expect(sql).toBe("SHOW CREATE FUNCTION `my.db`.`calc.totals`");
});
it("builds a quoted MariaDB SHOW CREATE PROCEDURE statement", () => {
const sql = buildShowCreateSQL(
"mariadb",
"PROCEDURE",
"test schema",
"get-data",
);
expect(sql).toBe("SHOW CREATE PROCEDURE `test schema`.`get-data`");

Copilot uses AI. Check for mistakes.
@tianzhou tianzhou merged commit bbf35be into bytebase:main Apr 3, 2026
5 of 6 checks passed
@tianzhou
Copy link
Copy Markdown
Member

tianzhou commented Apr 3, 2026

Thanks for the contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants