Skip to content

Commit c9ee003

Browse files
iancooperclaude
andcommitted
fix: PR #4039 F1c — MySQL fresh install honors SchemaName end-to-end (builder + V2..V7 ALTERs + detection)
PR #4039 reviewer item M4-1 (comment 4485697019) — MySQL half of F1. In MySQL "schema" is synonymous with "database". The pre-F1c behaviour: - MySqlOutboxBuilder / MySqlInboxBuilder GetDDL emitted unqualified `CREATE TABLE IF NOT EXISTS {table}`, landing the table in DATABASE() (the connection's bound database). - V2..V7 ALTERs used unqualified `{table}` and probed `information_schema.columns WHERE table_schema = DATABASE()`. - MySqlBoxDetectionHelper used the configured SchemaName for box-table existence checks AND for the history-table existence check. When SchemaName != connection.Database (a legitimate multi-tenant scenario), the box table landed in DATABASE() while detection looked in SchemaName — the silent split the reviewer flagged. A second run reported "no table" and re-ran fresh install, producing PK conflicts on history insert. Fix (Full schema-aware approach per user direction): 1. MySqlOutboxBuilder.GetDDL + MySqlInboxBuilder.GetDDL gain optional `schemaName` parameter — when non-null, emit `\`schema\`.\`table\`` (backticked); otherwise unqualified as before. 2. MySqlOutboxMigrationCatalog + MySqlInboxMigrationCatalog: - FreshInstallDdl threads configuration.SchemaName through to the builder with defence-in-depth Identifiers.AssertSafe. - V2..V7 ALTERs (via AddColumn helpers) take an optional `schema` parameter. When non-null, the table_schema probe filter and the ALTER target are both schema-qualified; when null, original behaviour (DATABASE() / unqualified) is preserved bit-for-bit. 3. MySqlBoxDetectionHelper.DoesHistoryExistAsync — the history table itself always lives in connection.Database (the runner's EnsureHistoryTableAsync emits unqualified DDL targeting DATABASE()), so the table-existence probe now uses connection.Database. The schemaName parameter still scopes the ROW lookup via the SchemaName column filter. Comment updated. V1HistoricalDdl on both catalogs stays unqualified (historical literal — pre- SchemaName-era). The unusual chain-bootstrap-at-V1 path with non-default SchemaName is not exercised in practice; documented constraint. Integration tests: - When_mysql_outbox_provisioner_runs_on_fresh_database_with_non_default_schema_it_should_create_in_configured_schema - When_mysql_inbox_provisioner_runs_on_fresh_database_with_non_default_schema_it_should_create_in_configured_schema Both pre-create a non-default database, configure SchemaName to it, run the provisioner twice, and assert (a) table lands in configured database (not BrighterTests/connection-default), (b) second run is idempotent, (c) history records exactly one V_latest row. Pre-fix: outbox test failed at "must exist in 'brighter_billing_*'"; after the builder+ALTER fix, detection bug surfaced as PK conflict on history insert (second run). Post-detection-fix: both tests GREEN. Full MySQL BoxProvisioning suite: 76/76 GREEN net9.0 (+2 vs the 74 post-F2 baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2c71925 commit c9ee003

7 files changed

Lines changed: 442 additions & 34 deletions

src/Paramore.Brighter.BoxProvisioning.MySql/MySqlBoxDetectionHelper.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,26 @@ SELECT EXISTS(SELECT 1 FROM information_schema.tables
7575
/// Returns true if the migration history table exists and has at least one row for the given
7676
/// box table.
7777
/// </summary>
78-
/// <param name="schemaName">Optional. Null is substituted with <c>connection.Database</c> per ADR 0057 §A.1.</param>
78+
/// <param name="schemaName">
79+
/// Optional. Filters the history rows by their <c>SchemaName</c> column. Null is substituted
80+
/// with <c>connection.Database</c> per ADR 0057 §A.1. Note: this parameter scopes the
81+
/// content lookup, NOT the existence check for the history table itself — the history table
82+
/// always lives in the connection's bound database, even when the box table is provisioned
83+
/// in a different schema (per PR #4039 F1c).
84+
/// </param>
7985
/// <param name="transaction">Accepted and ignored — MySQL DDL auto-commits per ADR 0057 §5a.</param>
8086
public async Task<bool> DoesHistoryExistAsync(
8187
MySqlConnection connection, string tableName, string? schemaName,
8288
CancellationToken cancellationToken = default,
8389
MySqlTransaction? transaction = null)
8490
{
91+
// The history table always resides in connection.Database (the MySQL runner's
92+
// EnsureHistoryTableAsync emits an unqualified CREATE TABLE IF NOT EXISTS targeting
93+
// DATABASE()). Probe its existence against connection.Database — not the
94+
// box-table SchemaName — so a non-default SchemaName configuration (F1c) does not
95+
// misreport HistoryAbsent and trigger a redundant bootstrap path.
8596
var historyTableExists = await DoesTableExistAsync(
86-
connection, "__BrighterMigrationHistory", schemaName, cancellationToken);
97+
connection, "__BrighterMigrationHistory", connection.Database, cancellationToken);
8798
if (!historyTableExists)
8899
return false;
89100

src/Paramore.Brighter.BoxProvisioning.MySql/MySqlInboxMigrationCatalog.cs

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,19 @@ public string FreshInstallDdl(IAmARelationalDatabaseConfiguration configuration)
9696
Identifiers.AssertSafe(
9797
configuration.InBoxTableName,
9898
nameof(IAmARelationalDatabaseConfiguration.InBoxTableName));
99-
return MySqlInboxBuilder.GetDDL(configuration.InBoxTableName, configuration.BinaryMessagePayload);
99+
// Pass SchemaName so the builder schema-qualifies the CREATE TABLE. Per PR #4039
100+
// reviewer item M4-1 (F1c). See MySqlOutboxMigrationCatalog for the full rationale.
101+
if (configuration.SchemaName is not null)
102+
{
103+
Identifiers.AssertSafe(
104+
configuration.SchemaName,
105+
nameof(IAmARelationalDatabaseConfiguration.SchemaName));
106+
}
107+
return MySqlInboxBuilder.GetDDL(
108+
configuration.InBoxTableName,
109+
configuration.BinaryMessagePayload,
110+
jsonMessage: false,
111+
schemaName: configuration.SchemaName);
100112
}
101113

102114
/// <summary>
@@ -107,8 +119,13 @@ public string FreshInstallDdl(IAmARelationalDatabaseConfiguration configuration)
107119
public IReadOnlyList<IAmABoxMigration> All(IAmARelationalDatabaseConfiguration configuration)
108120
{
109121
var table = configuration.InBoxTableName;
122+
var schema = configuration.SchemaName;
110123

111124
Identifiers.AssertSafe(table, nameof(IAmARelationalDatabaseConfiguration.InBoxTableName));
125+
if (schema is not null)
126+
{
127+
Identifiers.AssertSafe(schema, nameof(IAmARelationalDatabaseConfiguration.SchemaName));
128+
}
112129

113130
return
114131
[
@@ -121,7 +138,7 @@ public IReadOnlyList<IAmABoxMigration> All(IAmARelationalDatabaseConfiguration c
121138
new BoxMigration(
122139
Version: 2,
123140
Description: "Add ContextKey column",
124-
UpScript: AddColumn(table, "ContextKey", "VARCHAR(256)"),
141+
UpScript: AddColumn(schema, table, "ContextKey", "VARCHAR(256)"),
125142
LogicalColumns: Cumulative(2),
126143
SourceReference: "787c31c52")
127144
];
@@ -137,19 +154,24 @@ private static IReadOnlyCollection<string> Cumulative(int upToVersion)
137154

138155
/// <summary>
139156
/// MySQL 5.7+ idempotent ADD COLUMN — runtime <c>information_schema.columns</c> probe drives
140-
/// a prepared-statement that conditionally emits the ALTER. Runs against
141-
/// <c>DATABASE()</c> (the connection's bound schema), matching the un-qualified ALTER target.
142-
/// The added column is <c>NULL</c>-able — required because MySQL ADD COLUMN against a
143-
/// non-empty table must permit NULL or supply a DEFAULT, and we make no assumption about
144-
/// emptiness during bootstrap.
157+
/// a prepared-statement that conditionally emits the ALTER. When <paramref name="schema"/>
158+
/// is null, the probe targets <c>DATABASE()</c> and the ALTER is unqualified — original
159+
/// behaviour preserved. When non-null, both probe and ALTER are pinned to the configured
160+
/// schema (per PR #4039 reviewer item M4-1 / F1c). The added column is <c>NULL</c>-able —
161+
/// required because MySQL ADD COLUMN against a non-empty table must permit NULL or supply
162+
/// a DEFAULT, and we make no assumption about emptiness during bootstrap.
145163
/// </summary>
146-
private static string AddColumn(string table, string column, string type) =>
147-
$@"SET @q = (SELECT IF(
164+
private static string AddColumn(string? schema, string table, string column, string type)
165+
{
166+
var tableScopeForProbe = schema is null ? "DATABASE()" : $"'{schema}'";
167+
var qualifiedAlterTarget = schema is null ? $"`{table}`" : $"`{schema}`.`{table}`";
168+
return $@"SET @q = (SELECT IF(
148169
(SELECT COUNT(*) FROM information_schema.columns
149-
WHERE table_schema = DATABASE() AND table_name = '{table}' AND column_name = '{column}') = 0,
150-
'ALTER TABLE `{table}` ADD COLUMN `{column}` {type} NULL',
170+
WHERE table_schema = {tableScopeForProbe} AND table_name = '{table}' AND column_name = '{column}') = 0,
171+
'ALTER TABLE {qualifiedAlterTarget} ADD COLUMN `{column}` {type} NULL',
151172
'SELECT 1'));
152173
PREPARE stmt FROM @q;
153174
EXECUTE stmt;
154175
DEALLOCATE PREPARE stmt;";
176+
}
155177
}

src/Paramore.Brighter.BoxProvisioning.MySql/MySqlOutboxMigrationCatalog.cs

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,21 @@ public string FreshInstallDdl(IAmARelationalDatabaseConfiguration configuration)
9797
Identifiers.AssertSafe(
9898
configuration.OutBoxTableName,
9999
nameof(IAmARelationalDatabaseConfiguration.OutBoxTableName));
100-
return MySqlOutboxBuilder.GetDDL(configuration.OutBoxTableName, configuration.BinaryMessagePayload);
100+
// In MySQL "schema" is synonymous with "database". When SchemaName is set, the
101+
// builder schema-qualifies the CREATE TABLE so the table lands in the configured
102+
// database rather than the connection's bound DATABASE(). V2..V7 ALTERs below pick
103+
// up the same qualification via the `schema` parameter on AddColumns. Per PR #4039
104+
// reviewer item M4-1 (F1c).
105+
if (configuration.SchemaName is not null)
106+
{
107+
Identifiers.AssertSafe(
108+
configuration.SchemaName,
109+
nameof(IAmARelationalDatabaseConfiguration.SchemaName));
110+
}
111+
return MySqlOutboxBuilder.GetDDL(
112+
configuration.OutBoxTableName,
113+
configuration.BinaryMessagePayload,
114+
configuration.SchemaName);
101115
}
102116

103117
/// <summary>
@@ -108,8 +122,13 @@ public string FreshInstallDdl(IAmARelationalDatabaseConfiguration configuration)
108122
public IReadOnlyList<IAmABoxMigration> All(IAmARelationalDatabaseConfiguration configuration)
109123
{
110124
var table = configuration.OutBoxTableName;
125+
var schema = configuration.SchemaName;
111126

112127
Identifiers.AssertSafe(table, nameof(IAmARelationalDatabaseConfiguration.OutBoxTableName));
128+
if (schema is not null)
129+
{
130+
Identifiers.AssertSafe(schema, nameof(IAmARelationalDatabaseConfiguration.SchemaName));
131+
}
113132

114133
return
115134
[
@@ -122,14 +141,14 @@ public IReadOnlyList<IAmABoxMigration> All(IAmARelationalDatabaseConfiguration c
122141
new BoxMigration(
123142
Version: 2,
124143
Description: "Add Dispatched column",
125-
UpScript: AddColumns(table, ("Dispatched", "TIMESTAMP(3)")),
144+
UpScript: AddColumns(schema, table, ("Dispatched", "TIMESTAMP(3)")),
126145
LogicalColumns: Cumulative(2),
127146
SourceReference: "3c30343fa"),
128147

129148
new BoxMigration(
130149
Version: 3,
131150
Description: "Add CorrelationId, ReplyTo, ContentType columns",
132-
UpScript: AddColumns(table,
151+
UpScript: AddColumns(schema, table,
133152
("CorrelationId", "VARCHAR(255)"),
134153
("ReplyTo", "VARCHAR(255)"),
135154
("ContentType", "VARCHAR(128)")),
@@ -139,14 +158,14 @@ public IReadOnlyList<IAmABoxMigration> All(IAmARelationalDatabaseConfiguration c
139158
new BoxMigration(
140159
Version: 4,
141160
Description: "Add PartitionKey column",
142-
UpScript: AddColumns(table, ("PartitionKey", "VARCHAR(128)")),
161+
UpScript: AddColumns(schema, table, ("PartitionKey", "VARCHAR(128)")),
143162
LogicalColumns: Cumulative(4),
144163
SourceReference: "1cdc04b60 / #2560"),
145164

146165
new BoxMigration(
147166
Version: 5,
148167
Description: "Add CloudEvents columns (Source, Type, DataSchema, Subject, TraceParent, TraceState, Baggage)",
149-
UpScript: AddColumns(table,
168+
UpScript: AddColumns(schema, table,
150169
("Source", "VARCHAR(255)"),
151170
("Type", "VARCHAR(255)"),
152171
("DataSchema", "VARCHAR(255)"),
@@ -160,7 +179,7 @@ public IReadOnlyList<IAmABoxMigration> All(IAmARelationalDatabaseConfiguration c
160179
new BoxMigration(
161180
Version: 6,
162181
Description: "Add WorkflowId, JobId columns",
163-
UpScript: AddColumns(table,
182+
UpScript: AddColumns(schema, table,
164183
("WorkflowId", "VARCHAR(255)"),
165184
("JobId", "VARCHAR(255)")),
166185
LogicalColumns: Cumulative(6),
@@ -169,7 +188,7 @@ public IReadOnlyList<IAmABoxMigration> All(IAmARelationalDatabaseConfiguration c
169188
new BoxMigration(
170189
Version: 7,
171190
Description: "Add DataRef, SpecVersion columns",
172-
UpScript: AddColumns(table,
191+
UpScript: AddColumns(schema, table,
173192
("DataRef", "VARCHAR(255)"),
174193
("SpecVersion", "VARCHAR(255)")),
175194
LogicalColumns: Cumulative(7),
@@ -190,24 +209,32 @@ private static IReadOnlyCollection<string> Cumulative(int upToVersion)
190209
return set;
191210
}
192211

193-
private static string AddColumns(string table, params (string Column, string Type)[] columns) =>
194-
string.Join(Environment.NewLine, columns.Select(c => AddColumn(table, c.Column, c.Type)));
212+
private static string AddColumns(string? schema, string table, params (string Column, string Type)[] columns) =>
213+
string.Join(Environment.NewLine, columns.Select(c => AddColumn(schema, table, c.Column, c.Type)));
195214

196215
/// <summary>
197216
/// MySQL 5.7+ idempotent ADD COLUMN — runtime <c>information_schema.columns</c> probe drives
198-
/// a prepared-statement that conditionally emits the ALTER. Runs against
199-
/// <c>DATABASE()</c> (the connection's bound schema), matching the un-qualified ALTER target.
217+
/// a prepared-statement that conditionally emits the ALTER. When <paramref name="schema"/>
218+
/// is null, the probe targets <c>DATABASE()</c> (the connection's bound database) and the
219+
/// ALTER is unqualified — original behaviour preserved. When <paramref name="schema"/> is
220+
/// non-null, both the probe's <c>table_schema</c> filter and the ALTER target are pinned
221+
/// to that schema, so the chain reaches the box table created in the configured database
222+
/// (per PR #4039 reviewer item M4-1 / F1c).
200223
/// All added columns are <c>NULL</c>-able — required because MySQL ADD COLUMN against a
201224
/// non-empty table must permit NULL or supply a DEFAULT, and we make no assumption about
202225
/// emptiness during bootstrap.
203226
/// </summary>
204-
private static string AddColumn(string table, string column, string type) =>
205-
$@"SET @q = (SELECT IF(
227+
private static string AddColumn(string? schema, string table, string column, string type)
228+
{
229+
var tableScopeForProbe = schema is null ? "DATABASE()" : $"'{schema}'";
230+
var qualifiedAlterTarget = schema is null ? $"`{table}`" : $"`{schema}`.`{table}`";
231+
return $@"SET @q = (SELECT IF(
206232
(SELECT COUNT(*) FROM information_schema.columns
207-
WHERE table_schema = DATABASE() AND table_name = '{table}' AND column_name = '{column}') = 0,
208-
'ALTER TABLE `{table}` ADD COLUMN `{column}` {type} NULL',
233+
WHERE table_schema = {tableScopeForProbe} AND table_name = '{table}' AND column_name = '{column}') = 0,
234+
'ALTER TABLE {qualifiedAlterTarget} ADD COLUMN `{column}` {type} NULL',
209235
'SELECT 1'));
210236
PREPARE stmt FROM @q;
211237
EXECUTE stmt;
212238
DEALLOCATE PREPARE stmt;";
239+
}
213240
}

src/Paramore.Brighter.Inbox.MySql/MySqlInboxBuilder.cs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,19 +76,30 @@ PRIMARY KEY (`CommandId`)
7676
/// Gets the DDL statements to create an Inbox in MySQL
7777
/// </summary>
7878
/// <param name="inboxTableName">The Inbox Table Name</param>
79+
/// <param name="binaryMessage">Should the command body be stored as binary.</param>
80+
/// <param name="jsonMessage">Should the command body be stored using the JSON type.</param>
81+
/// <param name="schemaName">
82+
/// Optional MySQL schema (database) name. When non-null, the emitted DDL schema-qualifies
83+
/// the table as <c>`schemaName`.`inboxTableName`</c>; otherwise the table is emitted
84+
/// unqualified and lands in the connection's bound database. Per PR #4039 reviewer
85+
/// item M4-1 (F1c). See the outbox builder for the full rationale.
86+
/// </param>
7987
/// <returns></returns>
80-
public static string GetDDL(string inboxTableName, bool binaryMessage = false, bool jsonMessage = false)
88+
public static string GetDDL(string inboxTableName, bool binaryMessage = false, bool jsonMessage = false, string? schemaName = null)
8189
{
90+
var qualifiedTable = schemaName is null
91+
? inboxTableName
92+
: $"`{schemaName}`.`{inboxTableName}`";
8293
if (jsonMessage)
8394
{
84-
return string.Format(JsonInboxDDL, inboxTableName);
95+
return string.Format(JsonInboxDDL, qualifiedTable);
8596
}
8697
if (binaryMessage)
8798
{
88-
return string.Format(BinaryInboxxDDL, inboxTableName);
99+
return string.Format(BinaryInboxxDDL, qualifiedTable);
89100
}
90101

91-
return string.Format(TextInboxDDL, inboxTableName);
102+
return string.Format(TextInboxDDL, qualifiedTable);
92103
}
93104

94105
/// <summary>

src/Paramore.Brighter.Outbox.MySql/MySqlOutboxBuilder.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,25 @@ PRIMARY KEY (`MessageId`)
104104
/// </summary>
105105
/// <param name="outboxTableName">The name of the table to store messages in</param>
106106
/// <param name="hasBinaryMessagePayload">Should the message body be stored as binary? Conversion of binary data to/from UTF-8 is lossy</param>
107+
/// <param name="schemaName">
108+
/// Optional MySQL schema (database) name. When non-null, the emitted DDL schema-qualifies
109+
/// the table as <c>`schemaName`.`outboxTableName`</c>; otherwise the table is emitted
110+
/// unqualified and lands in the connection's bound database (<c>DATABASE()</c>). In MySQL
111+
/// "schema" is synonymous with "database"; per PR #4039 reviewer item M4-1 (F1c),
112+
/// configuring <see cref="Paramore.Brighter.IAmARelationalDatabaseConfiguration.SchemaName"/>
113+
/// to a database different from the connection's bound database requires schema-qualified
114+
/// DDL — without it, detection looks in SchemaName while creation lands in DATABASE().
115+
/// </param>
107116
/// <returns></returns>
108-
public static string GetDDL(string outboxTableName, bool hasBinaryMessagePayload = false)
117+
public static string GetDDL(string outboxTableName, bool hasBinaryMessagePayload = false, string? schemaName = null)
109118
{
110119
if (string.IsNullOrEmpty(outboxTableName))
111120
throw new ArgumentNullException(outboxTableName, $"You must provide a tablename for the OutBox table");
112121

113-
return string.Format(hasBinaryMessagePayload ? BinaryOutboxDdl : TextOutboxDdl, outboxTableName);
122+
var qualifiedTable = schemaName is null
123+
? outboxTableName
124+
: $"`{schemaName}`.`{outboxTableName}`";
125+
return string.Format(hasBinaryMessagePayload ? BinaryOutboxDdl : TextOutboxDdl, qualifiedTable);
114126
}
115127

116128
/// <summary>

0 commit comments

Comments
 (0)