Page MenuHomePhabricator

Reading list schema updater doesn't support db prefixes properly
Open, Needs TriagePublicBUG REPORT

Description

I haven't verified this myself, but based on report at https://www.mediawiki.org/wiki/Project:Support_desk/Flow/2023/03#While_updating_to_MediaWiki_1.39.2,_gives_me_a_Error_1146

Expected behaviour: all schema (.sql) files should include appropriate db prefixes in them (i.e. /*_*/ ) so that update.php works even when using a db prefix.

Steps to reproduce:

  • install mediawiki 1.38 with a db prefix set in the installer
  • install reading list extension
  • upgrade mediawiki and reading list extension
  • run update.php
  • note db error

Event Timeline

Ammarpad subscribed.

It has been fixed in 1.40 (851694), but not in older releases.

Hello! @Bawolff can i be assigned this? Im new to more organized large scale open-source projects and would like to begin my contribution to them with wikimedia.

@Chickenleaf: Hi and thank you for your interest! Please see the previous comment here. In general, please check https://www.mediawiki.org/wiki/New_Developers (and its communication section). The page covers how to get started, assigning tasks, task status, how to find a codebase, how to create patches, where to ask general development questions and where to get help with setup problems, and how to ask good questions. Thanks a lot! No need to ping task authors. :)

Thank you yes!! Shall not tag the authors.

Change #1247556 had a related patch set uploaded (by Reeti; author: Reeti):

[mediawiki/extensions/ReadingLists@master] Add db prefix support to SQL schema files

https://gerrit.wikimedia.org/r/1247556

Hello, I have proposed a patch to resolve this issue. The changes include adding the missing /*_*/ db prefix placeholders to the sql/postgres/tables-generated.sql file. The MySQL and SQLite files were already correct and required no changes. I welcome any feedback or reviews at your convenience. Thank you.

Did you re-run maintenance/generateSchemaSql.php or add it manually? Since this is a generated file, the fix needs to be somewhere in the pgSQL generator logic in core (and chances are it happened a long time ago and just no one re-ran the script - ReadingLists doesn't see a lot of changes these days).

Thank you for the feedback! The changes were added manually. Could you guide me on the correct approach should I re-run maintenance/generateSchemaSql.php to regenerate the file, or is there something else that needs to be fixed first? I'm happy to redo this the right way.

Try running the script (see docs) and since it will overwrite the file, you'll see if something else needs to be fixed or not.

I ran maintenance/generateSchemaSql.php and it only regenerated the MySQL file with a minor comment change (absolute path). The MySQL and SQLite files already had /*_*/ correctly. Only the postgres file was missing the prefixes, which I added manually. Happy to hear if a different approach is preferred!

Sorry for the lack of clarity. You need to run it for pgSQL specifically (so --json /path/to/tables.json --sql /path/to/postgres/tables-generated.sql --type postgres)

Thanks for the clarification! I ran the script with --type postgres and it confirmed that the /*_*/ prefixes I added manually are correct the script produces the same output. The only difference was an absolute path in the source comment line, which I reverted. The patch should be good to go!

Change #1247556 merged by jenkins-bot:

[mediawiki/extensions/ReadingLists@master] Add db prefix support to SQL schema files

https://gerrit.wikimedia.org/r/1247556