Repository navigation
Update to MusicBrainz schema 31 (rebase of #26) - #27
Conversation
note that this reverts the adaptions done by #8 as musicbrainz does not ship the scripts with function definitions containing a `SET schema` local adaptions would always lead to merge problems. if users have problems that functions do not use the correct schema for queries and can't find functions or tables, consider to set the schema on session, user or database level: ``` -- session SET search_path to musicbrainz,public; -- user level ALTER USER musicbrainz SET search_path TO musicbrainz, public; -- check user level settings via psql \drds mbdata; -- check user level via query SELECT r.rolname AS role_name, rs.setconfig AS configured_settings FROM pg_roles r LEFT JOIN pg_db_role_setting rs ON r.oid = rs.setrole WHERE r.rolname = 'musicbrainz'; -- db level SELECT setting FROM pg_settings WHERE name = 'search_path'; ALTER DATABASE database_name SET search_path TO musicbrainz, public; ```
musicbrainz has fixed upgrade issues see metabrainz/musicbrainz-server@c7b8c29
# Conflicts: # CHANGELOG.rst # mbslave/__init__.py # mbslave/sql/CreateFunctions.sql
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughChangesSchema 31 and replication
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mbslave/sql/updates/20260121-mbs-14092.sql`:
- Around line 79-81: Update the series_type seed INSERT for IDs 16 and 17 to use
the file’s established ON CONFLICT DO NOTHING pattern, preserving both rows’
values. Also update the migration’s SetSequences handling so the series_type ID
sequence is advanced beyond 17 and later inserts cannot collide.
- Around line 15-27: Update the series_series view’s link_type gid filter to use
the series→series part-of gid 8fe04b66-fe39-40ce-a28f-76b816d3f55a in
mbslave/sql/updates/20260121-mbs-14092.sql lines 15-27,
mbslave/sql/CreateViews.sql lines 86-98, and
mbslave/sql/updates/schema-change/31.all.sql lines 24-36; make the same
replacement in each view definition.
In `@mbslave/sql/updates/20260502-search-756-master_only.sql`:
- Around line 22-28: The pdcursor parameter in every recordchange() copy must
use BIGINT to match the BIGINT oooseqid and pending_data.seqid types. Update
mbslave/sql/updates/20260502-search-756-master_only.sql:22-28,
mbslave/sql/updates/20260507-search-756-mirror_only.sql:22-28, and
mbslave/sql/updates/20260507-search-756-standalone_only.sql:25-31; then
regenerate mbslave/sql/updates/schema-change/31.master_only.sql:28-34 and
mbslave/sql/updates/schema-change/31.mirror_only.sql:28-34 from the corrected
source.
In `@mbslave/sql/updates/schema-change/31.standalone_only.sql`:
- Around line 29-37: Change the pdcursor parameter type from INTEGER to BIGINT
in the standalone schema-change SQL and the corresponding dbmirror2 SQL
definition, while leaving the oooseqid declaration and cursor query behavior
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7e200039-0697-404c-933b-c1feb8a18eb7
📒 Files selected for processing (36)
CHANGELOG.rstREADME.rstmbslave/__init__.pymbslave/replication.pymbslave/sql/CreateAllReplicationTriggers2.sqlmbslave/sql/CreateConstraints.sqlmbslave/sql/CreateCustomReplicationTriggers2.sqlmbslave/sql/CreateFunctions.sqlmbslave/sql/CreateReplicationTriggers2.sqlmbslave/sql/CreateTables.sqlmbslave/sql/CreateViews.sqlmbslave/sql/DropAllReplicationTriggers2.sqlmbslave/sql/DropCustomReplicationTriggers2.sqlmbslave/sql/DropFunctions.sqlmbslave/sql/DropMirrorOnlyTriggers.sqlmbslave/sql/DropReplicationTriggers2.sqlmbslave/sql/DropTriggers.sqlmbslave/sql/DropViews.sqlmbslave/sql/dbmirror2/READMEmbslave/sql/dbmirror2/RefreshColumnInfo.sqlmbslave/sql/dbmirror2/ReplicationSetup.sqlmbslave/sql/dbmirror2/dbmirror2.sqlmbslave/sql/updates/20260121-mbs-14092-fks.sqlmbslave/sql/updates/20260121-mbs-14092.sqlmbslave/sql/updates/20260212-mbs-14252.sqlmbslave/sql/updates/20260422-mbs-6551-all.sqlmbslave/sql/updates/20260422-mbs-6551-master_and_standalone.sqlmbslave/sql/updates/20260502-search-756-all.sqlmbslave/sql/updates/20260502-search-756-master_only.sqlmbslave/sql/updates/20260507-search-756-mirror_only.sqlmbslave/sql/updates/20260507-search-756-standalone_only.sqlmbslave/sql/updates/schema-change/31.all.sqlmbslave/sql/updates/schema-change/31.master_and_standalone.sqlmbslave/sql/updates/schema-change/31.master_only.sqlmbslave/sql/updates/schema-change/31.mirror_only.sqlmbslave/sql/updates/schema-change/31.standalone_only.sql
💤 Files with no reviewable changes (2)
- mbslave/sql/dbmirror2/RefreshColumnInfo.sql
- mbslave/sql/dbmirror2/ReplicationSetup.sql
| CREATE OR REPLACE VIEW series_series AS | ||
| SELECT entity0 AS series_part, | ||
| entity1 AS series, | ||
| lss.id AS relationship, | ||
| link_order, | ||
| lss.link, | ||
| COALESCE(text_value, '') AS text_value | ||
| FROM l_series_series lss | ||
| JOIN series s ON s.id = lss.entity1 | ||
| JOIN link l ON l.id = lss.link | ||
| JOIN link_type lt ON (lt.id = l.link_type AND lt.gid = '8da75c99-46ff-373c-9d31-276ca8fa8cc3') | ||
| LEFT OUTER JOIN link_attribute_text_value latv ON (latv.attribute_type = 788 AND latv.link = l.id) | ||
| ORDER BY series, link_order; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a '^(20260121-mbs-14092|CreateViews|31\.all|31\.master_only|31\.mirror_only|.*search-756.*)\.sql$' . | sed 's#^\./##' | sort
echo
echo "== search gid occurrences =="
rg -n "8da75c99|8fe04b66|SERIES_PART_OF_SERIES_GID|series_series|part of|Link:.*series" . -g '*.sql' -g '*.json' || true
echo
echo "== inspect candidate files around views and seed/DDL references =="
for f in \
mbslave/sql/updates/20260121-mbs-14092.sql \
mbslave/sql/CreateViews.sql \
mbslave/sql/updates/schema-change/31.all.sql
do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f"
fi
done
echo
echo "== list scripts mentioning 1307 link_type and series =="
rg -n "1307|series|series_series|link_type" mbslave/sql -g '*.sql' | head -n 200Repository: acoustid/mbslave
Length of output: 50372
🌐 Web query:
MusicBrainz link_type gid '8fe04b66-fe39-40ce-a28f-76b816d3f55a' series series part of
💡 Result:
The MusicBrainz link_type GID 8fe04b66-fe39-40ce-a28f-76b816d3f55a is not identified in the provided search results as a recognized relationship type in the MusicBrainz database. MusicBrainz uses GIDs (globally unique identifiers) to uniquely identify relationship types [1][2], and these types are documented in specific relationship type pages, such as those for Series-Series, Release-Series, and Release group-Series relationships [3][4][5]. While these documented relationship types cover various ways entities are linked—including "part of" relationships for releases and release groups [4][5]—the specific GID provided does not appear in the available documentation or search results.
Citations:
- 1: https://musicbrainz.readthedocs.io/en/latest/musicbrainz_database/schema/_schema.html
- 2: https://musicbrainz.org/doc/MusicBrainz_Database/Schema%23Track
- 3: https://musicbrainz.org/relationships/series-series
- 4: https://musicbrainz.org/relationships/release-series
- 5: https://musicbrainz.org/relationships/release_group-series
🌐 Web query:
MusicBrainz link_type gid '8da75c99-46ff-373c-9d31-276ca8fa8cc3'
💡 Result:
In the MusicBrainz database, the Global Identifier (GID) '8da75c99-46ff-373c-9d31-276ca8fa8cc3' corresponds to a link type related to the 'series' entity [1]. Specifically, it is defined in the MusicBrainz codebase as the link type for the 'series' entity itself within the context of 'part of series' relationships [1]. This GID is used to identify this specific relationship type within the link_type table of the MusicBrainz database schema [2][1].
Citations:
- 1: https://github.com/metabrainz/musicbrainz-server/blob/master/lib/MusicBrainz/Server/Constants.pm
- 2: https://musicbrainz.org/doc/MusicBrainz_Database/Schema
🌐 Web query:
MusicBrainz Server SeriesPartOf SeriesGID 8fe04b66-fe39-40ce-a28f-76b816d3f55a
💡 Result:
The identifier 8fe04b66-fe39-40ce-a28f-76b816d3f55a is a MusicBrainz GUID (Globally Unique Identifier), also known as an MBID, which specifically identifies a Series entity within the MusicBrainz database [1][2]. In the MusicBrainz data model, a Series represents a sequence of related entities—such as release groups, releases, recordings, works, artists, or events—that share a common theme or branding [2]. Each series is assigned a unique MBID (the GUID) to distinguish it from other entities in the system [1]. While your query references SeriesPartOf and SeriesGID, please note the following technical details regarding MusicBrainz terminology: - SeriesGID: This is a common way to refer to the Global Unique Identifier (GID) of a Series entity [1][3]. - SeriesPartOf: There is no standard "SeriesPartOf" entity type or property in MusicBrainz. Instead, relationships are used to define connections between entities [4]. For example, the relationship type "part of" is commonly used to indicate that a release group belongs to a specific series [5]. You can look up the specific series associated with this identifier by appending the GUID to the MusicBrainz series URL: https://musicbrainz.org/series/8fe04b66-fe39-40ce-a28f-76b816d3f55a [1][3].
Citations:
- 1: https://community.metabrainz.org/t/what-is-the-guid-at-the-end-of-the-musicbrainz-url/765369
- 2: https://musicbrainz.org/doc/Series
- 3: https://www.wikidata.org/wiki/Property:P1407
- 4: http://musicbrainz.org/doc/How_to_Add_a_Series
- 5: https://musicbrainz.org/relationships/release_group-series
🌐 Web query:
MusicBrainz Server Constants.pm 8da75c99-46ff-373c-9d31-276ca8fa8cc3
💡 Result:
In the MusicBrainz server codebase, the file lib/MusicBrainz/Server/Constants.pm is a Perl module used to define various constants and identifiers that have specific meanings within the database [1][2]. These constants include row IDs and Globally Unique Identifiers (GIDs) for special entities and system-level configuration parameters [1][2]. The UUID 8da75c99-46ff-373c-9d31-276ca8fa8cc3 is defined in the %PART_OF_SERIES hash within Constants.pm [1]. It specifically represents the Series entity type within the MusicBrainz database [1]. This mapping allows the server logic to programmatically identify and handle Series objects throughout the application [1][3]. The module typically contains: - Mappings for special entities (e.g., "Various Artists," "[no label]," "Deleted Artist") [1][2]. - Entity type GIDs (as seen with the Series GID) [1]. - Configuration for system behaviors, such as voting periods, edit listing limits, and sanitized dump generation [2][4]. By centralizing these values in Constants.pm, the MusicBrainz server ensures consistency when interacting with the database or performing entity-specific operations across different parts of the application [2][5].
Citations:
- 1: https://github.com/metabrainz/musicbrainz-server/blob/master/lib/MusicBrainz/Server/Constants.pm
- 2: https://github.com/metabrainz/musicbrainz-server/blob/29b6c43f/lib/MusicBrainz/Server/Constants.pm
- 3: https://github.com/metabrainz/musicbrainz-server/blob/master/admin/sql/CreateViews.sql
- 4: MBS-14183: Generate placeholder email for editor sanitized dumps metabrainz/musicbrainz-server#3663
- 5: https://github.com/metabrainz/musicbrainz-server/blob/master/lib/MusicBrainz/Server/Data/Utils.pm
🌐 Web query:
MusicBrainz 8fe04b66-fe39-40ce-a28f-76b816d3f55a link_type series-series part of admin/sql/CreateViews.sql
💡 Result:
The MusicBrainz identifier 8fe04b66-fe39-40ce-a28f-76b816d3f55a corresponds to a specific link type in the MusicBrainz database [1][2]. Specifically, this UUID identifies the "part of" relationship type used within series-to-series links [3][1]. In the MusicBrainz database schema, the link_type table stores the definitions for relationship types [1][2]. The file admin/sql/CreateViews.sql in the musicbrainz-server repository is a SQL script used to define database views, which often join these link type tables with their corresponding relationship tables (such as l_series_series) to make querying easier [4]. Summary of key technical points: - Identifier: 8fe04b66-fe39-40ce-a28f-76b816d3f55a is the unique global identifier (GID) for the "part of" link type [3][1]. - Relationship: It is typically utilized in l_series_series relationships to indicate that one series is a part of another [3][5]. - Database Context: admin/sql/CreateViews.sql is part of the server infrastructure that manages how these relationship types are exposed and queried via SQL views [4].
Citations:
- 1: https://musicbrainz-docs-development.readthedocs.io/en/latest/musicbrainz%5Fdatabase/schema/_schema.html
- 2: https://musicbrainz.org/doc/MusicBrainz_Database/Schema%23Track
- 3: https://musicbrainz.org/relationships/series-series
- 4: https://github.com/metabrainz/musicbrainz-server/blob/master/admin/sql/CreateViews.sql
- 5: https://musicbrainz.org/doc/Replication
🌐 Web query:
MusicBrainz 8da75c99-46ff-373c-9d31-276ca8fa8cc3 admin/sql/CreateViews.sql series_series
💡 Result:
In the MusicBrainz database schema, series_series is a database view defined within the project's SQL administration scripts, specifically located in CreateViews.sql [1]. This view is designed to facilitate queries involving series-to-series relationships [1]. It typically joins tables such as l_series_series (the link table for series-to-series relationships), series (the main entity table), and associated link and attribute tables to allow for the retrieval of hierarchical or associative series data [1]. The reference 8da75c99-46ff-373c-9d31-276ca8fa8cc3 appears to be a commit hash or a specific identifier within the MusicBrainz server repository context [2], related to the management of these schema objects. The MusicBrainz database uses a series of SQL scripts found in the admin/sql directory of the musicbrainz-server codebase to manage the schema, including the creation of views, constraints, and tables [2][3][4][5]. Series entities in MusicBrainz represent sequences of release groups, releases, recordings, works, or events that share a common theme, often with numbering to indicate their position within the series [5].
Citations:
- 1: Update to schema v31 #26
- 2: https://github.com/metabrainz/musicbrainz-server/blob/master/admin/sql/CreateViews.sql
- 3: https://github.com/metabrainz/musicbrainz-server/tree/master/admin/sql
- 4: https://github.com/metabrainz/musicbrainz-server/blob/master/admin/InitDb.pl
- 5: https://musicbrainz.org/doc/MusicBrainz_Database/Schema
Use the part-of link_type gid in series_series.
8fe04b66-fe39-40ce-a28f-76b816d3f55a is defined for the series→series “part of” rel, while 8da75c99-46ff-373c-9d31-276ca8fa8cc3 is not the relation type this view is filtering. The view joins lt.gid, so the existing CREATE OR REPLACE VIEW copies become empty while the new rel rows use the other gid.
mbslave/sql/updates/20260121-mbs-14092.sql#L25mbslave/sql/CreateViews.sql#L96mbslave/sql/updates/schema-change/31.all.sql#L34
📍 Affects 3 files
mbslave/sql/updates/20260121-mbs-14092.sql#L15-L27(this comment)mbslave/sql/CreateViews.sql#L86-L98mbslave/sql/updates/schema-change/31.all.sql#L24-L36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mbslave/sql/updates/20260121-mbs-14092.sql` around lines 15 - 27, Update the
series_series view’s link_type gid filter to use the series→series part-of gid
8fe04b66-fe39-40ce-a28f-76b816d3f55a in
mbslave/sql/updates/20260121-mbs-14092.sql lines 15-27,
mbslave/sql/CreateViews.sql lines 86-98, and
mbslave/sql/updates/schema-change/31.all.sql lines 24-36; make the same
replacement in each view definition.
| INSERT INTO series_type (id, name, entity_type, parent, child_order, description, gid) VALUES | ||
| (16, 'Series series', 'series', NULL, 6, 'A series of series.', generate_uuid_v3('6ba7b8119dad11d180b400c04fd430c8', 'series_type16')), | ||
| (17, 'Series award', 'series', 16, 0, 'A series of series (such as podcasts or festivals) honoured by the same award.', generate_uuid_v3('6ba7b8119dad11d180b400c04fd430c8', 'series_type17')); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
These inserts aren't idempotent, unlike every other insert in the file.
Every other seed statement here uses ON CONFLICT DO NOTHING; these two don't. A standalone DB that already carries series_type 16/17 (or the gids) will abort the whole upgrade transaction. Also check whether the series_type id sequence gets bumped past 17 somewhere (SetSequences), otherwise later inserts collide.
🔧 Match the surrounding style
INSERT INTO series_type (id, name, entity_type, parent, child_order, description, gid) VALUES
(16, 'Series series', 'series', NULL, 6, 'A series of series.', generate_uuid_v3('6ba7b8119dad11d180b400c04fd430c8', 'series_type16')),
- (17, 'Series award', 'series', 16, 0, 'A series of series (such as podcasts or festivals) honoured by the same award.', generate_uuid_v3('6ba7b8119dad11d180b400c04fd430c8', 'series_type17'));
+ (17, 'Series award', 'series', 16, 0, 'A series of series (such as podcasts or festivals) honoured by the same award.', generate_uuid_v3('6ba7b8119dad11d180b400c04fd430c8', 'series_type17'))
+ ON CONFLICT DO NOTHING;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| INSERT INTO series_type (id, name, entity_type, parent, child_order, description, gid) VALUES | |
| (16, 'Series series', 'series', NULL, 6, 'A series of series.', generate_uuid_v3('6ba7b8119dad11d180b400c04fd430c8', 'series_type16')), | |
| (17, 'Series award', 'series', 16, 0, 'A series of series (such as podcasts or festivals) honoured by the same award.', generate_uuid_v3('6ba7b8119dad11d180b400c04fd430c8', 'series_type17')); | |
| INSERT INTO series_type (id, name, entity_type, parent, child_order, description, gid) VALUES | |
| (16, 'Series series', 'series', NULL, 6, 'A series of series.', generate_uuid_v3('6ba7b8119dad11d180b400c04fd430c8', 'series_type16')), | |
| (17, 'Series award', 'series', 16, 0, 'A series of series (such as podcasts or festivals) honoured by the same award.', generate_uuid_v3('6ba7b8119dad11d180b400c04fd430c8', 'series_type17')) | |
| ON CONFLICT DO NOTHING; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mbslave/sql/updates/20260121-mbs-14092.sql` around lines 79 - 81, Update the
series_type seed INSERT for IDs 16 and 17 to use the file’s established ON
CONFLICT DO NOTHING pattern, preserving both rows’ values. Also update the
migration’s SetSequences handling so the series_type ID sequence is advanced
beyond 17 and later inserts cannot collide.
| pdcursor NO SCROLL CURSOR (oooseqid INTEGER) FOR | ||
| SELECT seqid | ||
| FROM dbmirror2.pending_data | ||
| WHERE xid = txid_current() | ||
| AND seqid >= oooseqid | ||
| ORDER BY seqid DESC | ||
| FOR UPDATE; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
pdcursor (oooseqid INTEGER) narrows a BIGSERIAL sequence id in all five copies of recordchange(). oooseqid is BIGINT; passing it into an INTEGER cursor parameter throws "integer out of range" once pending_data.seqid crosses 2^31, which kills every write on the database. Same one-word fix everywhere (keys TEXT[] is also declared and unused in each copy).
mbslave/sql/updates/20260502-search-756-master_only.sql#L22-L28: change the cursor parameter toBIGINT.mbslave/sql/updates/20260507-search-756-mirror_only.sql#L22-L28: same change.mbslave/sql/updates/20260507-search-756-standalone_only.sql#L25-L31: same change.mbslave/sql/updates/schema-change/31.master_only.sql#L28-L34: regenerate from the corrected source.mbslave/sql/updates/schema-change/31.mirror_only.sql#L28-L34: regenerate from the corrected source.
If these bodies are meant to stay byte-identical to the upstream MusicBrainz scripts, say so and I'll drop this — but then it's worth reporting upstream.
📍 Affects 5 files
mbslave/sql/updates/20260502-search-756-master_only.sql#L22-L28(this comment)mbslave/sql/updates/20260507-search-756-mirror_only.sql#L22-L28mbslave/sql/updates/20260507-search-756-standalone_only.sql#L25-L31mbslave/sql/updates/schema-change/31.master_only.sql#L28-L34mbslave/sql/updates/schema-change/31.mirror_only.sql#L28-L34
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mbslave/sql/updates/20260502-search-756-master_only.sql` around lines 22 -
28, The pdcursor parameter in every recordchange() copy must use BIGINT to match
the BIGINT oooseqid and pending_data.seqid types. Update
mbslave/sql/updates/20260502-search-756-master_only.sql:22-28,
mbslave/sql/updates/20260507-search-756-mirror_only.sql:22-28, and
mbslave/sql/updates/20260507-search-756-standalone_only.sql:25-31; then
regenerate mbslave/sql/updates/schema-change/31.master_only.sql:28-34 and
mbslave/sql/updates/schema-change/31.mirror_only.sql:28-34 from the corrected
source.
| oooseqid BIGINT; | ||
| oootrgdepth INTEGER; | ||
| pdcursor NO SCROLL CURSOR (oooseqid INTEGER) FOR | ||
| SELECT seqid | ||
| FROM dbmirror2.pending_data | ||
| WHERE xid = txid_current() | ||
| AND seqid >= oooseqid | ||
| ORDER BY seqid DESC | ||
| FOR UPDATE; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does PostgreSQL PL/pgSQL implicitly assignment-cast a bigint argument into an integer-typed cursor parameter, and raise "integer out of range" on overflow?
💡 Result:
Yes, PostgreSQL will raise an "integer out of range" error if you attempt to pass a value that exceeds the range of the integer type into a PL/pgSQL cursor parameter defined as an integer. When a cursor is declared with parameters, those parameters have explicitly defined data types [1]. When the cursor is opened, the provided argument values are substituted into the cursor's query [1]. If an argument provided to the OPEN statement is of a broader type (such as bigint) and it is passed into a parameter defined as a narrower type (such as integer), PostgreSQL will attempt an implicit cast [2][3]. Because implicit casting from a larger integer type (bigint) to a smaller integer type (integer) is not a "widening" conversion, the operation is subject to range checks. If the value of the bigint argument is outside the valid range for the integer type (i.e., less than -2,147,483,648 or greater than 2,147,483,647), the cast will fail, resulting in an "integer out of range" error [4][5]. This behavior is consistent with standard PostgreSQL assignment and casting rules, where down-casting is not permitted for values that do not fit into the target type [2][3].
Citations:
- 1: https://www.postgresql.org/docs/current/plpgsql-cursors.html
- 2: https://www.postgresql.org/docs/19/plpgsql-control-structures.html
- 3: https://www.postgresql.org/docs/17/plpgsql-control-structures.html
- 4: https://www.exchangetuts.com/postgres-getting-is-out-of-range-for-type-integer-when-using-nullif-1760210102973418
- 5: https://www.postgresql.org/message-id/1375322523041-5765833.post%40n5.nabble.com
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)31\.standalone_only\.sql$|(^|/)dbmirror2\.sql$' || true
echo
echo "== relevant file snippets =="
for f in mbslave/sql/updates/schema-change/31.standalone_only.sql mbslave/sql/dbmirror2/dbmirror2.sql; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
rg -n "oooseqid|pdcursor|seqid|recordchange|pending_data" "$f" -C 3
fi
done
echo
echo "== parse/scan cursor declare and call sites =="
python3 - <<'PY'
from pathlib import Path
for path in [Path("mbslave/sql/updates/schema-change/31.standalone_only.sql"), Path("mbslave/sql/dbmirror2/dbmirror2.sql")]:
if not path.exists():
continue
text = path.read_text()
print(f"\n== {path} ==")
for needle in ["oooseqid BIGINT", "pdcursor NO SCROLL CURSOR", "pdcursor(oooseqid := oooseqid)", "pdcursor (oooseqid := oooseqid)", "pdcursor(oooseqid := oooseqid)", "pdcursor(oooseqid"] :
if needle in text:
idx = text.index(needle)
start = max(0, text.rfind("\n", 0, idx-200))
end = min(len(text), text.find("\n", idx+2000))
print(f"NEEDLE {repr(needle)} found")
print(text[start:end])
PYRepository: acoustid/mbslave
Length of output: 21960
Use BIGINT for the pdcursor out-of-order seqid parameter.
oooseqid and pending_data.seqid are BIGINT/BIGSERIAL, so a sequence value above 2147483647 cannot be assigned to the cursor parameter without failing integer out of range; this can break the trigger path when the queue is still active. Apply the same change to mbslave/sql/dbmirror2/dbmirror2.sql to keep the upstream mirror SQL consistent.
Proposed fix
- pdcursor NO SCROLL CURSOR (oooseqid INTEGER) FOR
+ pdcursor NO SCROLL CURSOR (oooseqid BIGINT) FOR📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| oooseqid BIGINT; | |
| oootrgdepth INTEGER; | |
| pdcursor NO SCROLL CURSOR (oooseqid INTEGER) FOR | |
| SELECT seqid | |
| FROM dbmirror2.pending_data | |
| WHERE xid = txid_current() | |
| AND seqid >= oooseqid | |
| ORDER BY seqid DESC | |
| FOR UPDATE; | |
| oooseqid BIGINT; | |
| oootrgdepth INTEGER; | |
| pdcursor NO SCROLL CURSOR (oooseqid BIGINT) FOR | |
| SELECT seqid | |
| FROM dbmirror2.pending_data | |
| WHERE xid = txid_current() | |
| AND seqid >= oooseqid | |
| ORDER BY seqid DESC | |
| FOR UPDATE; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mbslave/sql/updates/schema-change/31.standalone_only.sql` around lines 29 -
37, Change the pdcursor parameter type from INTEGER to BIGINT in the standalone
schema-change SQL and the corresponding dbmirror2 SQL definition, while leaving
the oooseqid declaration and cursor query behavior unchanged.
1 participant
Rebases #26 (by @frisi, branch
MD-131-schema-update-v31on molindo/mbslave) on top of currentmain(post-#24) and resolves the resulting conflicts:mbslave/__init__.py/CHANGELOG.rst: kept both the 30.0.0 changelog entry from Optimize initial import and update to MusicBrainz schema 30 #24 and added 31.0.0 on top, version bumped to 31.0.0.mbslave/sql/CreateFunctions.sql: pure-addition conflict (new editor-data-sanitization functions), no logical conflict.No other conflicts.
py_compilepasses on all Python files.Closes #26.