Context

This functionality will allow event organizers to specify whether an event is a test event (and therefore if it should be surfaced in any public event tools such as the collaboration list).

Schema

ALTER TABLE /*_*/campaign_events

ADD event_is_test_event TINYINT(1) DEFAULT 0 NOT NULL;

This column is not indexed, while it is used to filter the events lookup on Special:AllEvents, this is paged so a limited number of records is pulled at once and an index did not feel necessary for this load.

The default is 0 rather than null as a null state is not representative of anything, and existing events should be marked as active rather than risking them being lost with a null value that has been overlooked.

@MHorsey-WMF please follow the procedure for the schema change creation described at https://wikitech.wikimedia.org/wiki/Schema_changes#Workflow_of_a_schema_change
It will allow us to get to it faster and in a more safer way. Thank you!

Apologies @Marostegui, I thought I was by adding the "schema_change" tag, but then somebody advised me to also add the DBA tag as well.

In T381759#10397461, @MHorsey-WMF wrote:

Apologies @Marostegui, I thought I was by adding the "schema_change" tag, but then somebody advised me to also add the DBA tag as well.

No worries. The procedure is described on the wikitech link above. We have it because we receive many of them, so having always the same template make it easier for us to review and get to it.
Thanks!

Hi @Marostegui! To clarify, this is not a request to perform the schema change just yet. We're reaching out for feedback on the schema change itself, and to let y'all know that this is coming so you can plan in advance :) Basically what the workflow documentation suggests with «Please include at least a DBA also ...».

@Daimona ah! As the syntax was included there I thought it was. I think it does make sense

On design of the schema change, one different schema could be to have a dedicated table for test events but given the number of events, I think this is fine and simpler.

Reiterating my comment from the gerrit patch. https://www.mediawiki.org/wiki/Manual:Schema_changes#Notes states that "boolean" is not a valid DBAL type for mediawiki, which is why the field was added as mwtinyint(1), but we have used boolean elsewhere. Is the documentation correct?

In T381759#10461785, @MHorsey-WMF wrote:

Reiterating my comment from the gerrit patch. https://www.mediawiki.org/wiki/Manual:Schema_changes#Notes states that "boolean" is not a valid DBAL type for mediawiki, which is why the field was added as mwtinyint(1), but we have used boolean elsewhere. Is the documentation correct?

Yes the documentation is correct, it probably means where you used boolean Postgres wouldn't work.

In T381759#10461839, @Ladsgroup wrote:

In T381759#10461785, @MHorsey-WMF wrote:

Reiterating my comment from the gerrit patch. https://www.mediawiki.org/wiki/Manual:Schema_changes#Notes states that "boolean" is not a valid DBAL type for mediawiki, which is why the field was added as mwtinyint(1), but we have used boolean elsewhere. Is the documentation correct?

Yes the documentation is correct, it probably means where you used boolean Postgres wouldn't work.

I see, thank you. We're already using booleans for ce_participants.cep_private, and I thought that would work on postgres. I have manually triggered tests on postgres and indeed, it looks like it isn't working. So, I see a few opportunities for improvement here:

• I would add a note to https://www.mediawiki.org/wiki/Manual:Schema_changes explaining why booleans are not allowed (like the existing notes about timestamps, varchar, and enum). With the postgres pointer above I found e.g. T298717, T257755#6863029, and T298720. Without the pointer, I wasn't even sure if lack of boolean support was intentional. Done.
• The postgres incompatibility seems to not really be an incompatibility, but more of an explicit choice on the MW side. DatabasePostgres::addQuotes (like its siblings) explicitly converts booleans to integers. My understanding is that this is done 1) for consistency with other DBMSs, and 2) for historical reasons (i.e., existing tinyint types that are using booleans in PHP). I do wonder if it still makes sense to have this type conversion nowadays with abstract schema, but I'll leave it at that for now.
• Schema validation is not currently failing for booleans. T298720 was originally created to track this, then it was implemented with abstract schema validation, then an exception was added in r817361 for more historical reasons, and never reverted. It would be nice to fix this, too. Filed T383776: Enforce rejection of "boolean" in abstract DB schema.
• From the postgres tests I ran, it's clear that multiple extensions are not compatible with postgres. It would be nice to actually test postgres compatibility in CI, as originally proposed in T313138. I have posted some stats in T313138#10462212.

Also:

• We should change ce_participants.cep_private to be a tinyint. Filed T383777: Convert `ce_participants.cep_private` from boolean to mwtinyint.
• Judging from my quick search, GrowthExperiments also has a boolean field. I'm not sure if they're aware of the whole postgres situation. Filed T383778: The `growthexperiments_mentor_mentee.gemm_mentee_is_active` DB column usees the "boolean" type, which is not supported with postgres.

Thanks for the investigation. I also hope after Flow's undeploy we can start testing for PG much better and avoid such situations in the future.

Nothing to review here. @MHorsey-WMF were you planning to use this task for https://wikitech.wikimedia.org/wiki/Schema_changes#Workflow_of_a_schema_change, or another one?

In T381759#10471545, @Daimona wrote:

Nothing to review here. @MHorsey-WMF were you planning to use this task for https://wikitech.wikimedia.org/wiki/Schema_changes#Workflow_of_a_schema_change, or another one?

@MHorsey-WMF See question above (also, I don't think there's ever going to be anything to review on this task, regardless of the answer).

@Daimona yes, I was planning to retain this ticket for that process.

@Daimona so this ticket is fully ready to be deployed in production?

In T381759#10482712, @Marostegui wrote:

@Daimona so this ticket is fully ready to be deployed in production?

Hi @Marostegui, yes it is.

Mentioned in SAL (#wikimedia-operations) [2025-01-22T10:59:56Z] <marostegui> Deploy schema change in codfw x1 with replication on the master dbmaint T381759

That's vey strange, I'll get this fixed

This should be fixed now.