Replace char and varchar with TEXT in Database #8372
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new migration entry in the unreleased migrations file and updates several SQL queries. It modifies the logic in the folder and user models to change query structure using Changes
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
frcroth
changed the title
frcroth
force-pushed
the
postgres-best-practices
branch
from
e30decb to
dda410c
Compare
frcroth
changed the title
|
Migration on WK data took 22 minutes on my machine. But nothing else happened in the browser or terminal until the migration was completed: |
There was a problem hiding this comment.
Hmm, 22 minutes doesn’t sound too great. On the plus side, the new code can run with the old database, so we won’t have to go into a full wk downtime for the duration of the migration. But then it looks like postgres will be very busy during the migration. I wonder if we could make this less impactful by running a transaction per table rather than a full one? But that might not work with the foreign key constraints 🤔
TBH I think if the migration creates too much work or downtime it might not be worth pursuing for us. Yes, the current data types have some weird quirks, as described in the linked document, but they haven’t caused any real problems for us yet.
@normanrz what do you think about this?
|
Can we do the migration before starting the new build in the running system? If yes, 22min doesn't sound too bad. But maybe worth doing it on a per-table basis to not lock the entire database. |
Yes, before and after should both be compatible. @frcroth could you have a look how well the migration could be split into multiple transactions? |
|
Could you please try to re-test this on a production db dump? the biggest tables are probably annotations and analyticsEvents |
|
For me locally with the production dump:
Everything else was < 1s |
|
Ok that actually sounds acceptable. Then I’d say we can go ahead with this. Reading through the new schema.sql, I’m wondering if we can encode our different types somehow. How do you feel about this, e.g. for our ObjectIds? https://stackoverflow.com/questions/73716345/how-to-define-type-nickname-in-in-postgresql |
|
I think we can run that some time in the morning. Maybe next Monday? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tools/postgres/schema.sql (1)
9-486: Consistent Data Type Conversion Across the Entire SchemaThis file contains a very comprehensive set of transaction blocks that alter nearly every table’s character-type columns from CHAR/VARCHAR to TEXT (or TEXT[]). The pattern is consistent:
• Each table’s changes are isolated in their own transaction, and associated views are dropped and recreated afterward.
• Default values and CHECK constraints appear to be preserved during these alterations.A couple of suggestions:
• Ensure that the application downtime or maintenance window is scheduled during this migration since the transient dropping of views could affect running queries.
• For future migrations, you might consider automating these repetitive tasks (for example, generating the ALTER statements from a metadata file) to improve maintainability.Overall, the changes are in line with PostgreSQL’s implicit conversion rules (so no USING clause is needed) and meet the PR objective.
conf/evolutions/128-replace-text-types.sql (3)
80-84: Duplicate ALTER Statement for 'mappings' ColumnWithin the transaction for the webknosos.dataset_layers table, the statement to alter the “mappings” column to TEXT[] appears twice consecutively. Removing the duplicate will streamline the script and reduce any potential confusion during execution.
279-286: Naming Consistency in MultiUsers Table AlterationsThere is a minor inconsistency in table naming within the multiUsers block. For example, most ALTER TABLE statements use “webknosos.multiUsers” while one statement at line 286 refers to “webknosos.multiusers” and the column name in lowercase (_lastloggedinidentity). Although PostgreSQL is case-insensitive with unquoted identifiers, maintaining consistent naming is best practice for clarity and future maintenance.
1-487: Thorough and Well-Segmented Migration ScriptThis migration script methodically converts all targeted CHAR/VARCHAR columns to TEXT across a vast number of tables. The structure—using separate transactions per table to drop/recreate views and update defaults—minimizes lock durations and enables partial completion if necessary. Before deploying to production, it’s advisable to re-test this script on a full production dump (as already mentioned in the PR objectives) to validate both performance and compatibility with existing queries and triggers.
Overall, the changes align well with PostgreSQL best practices and the PR objectives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
MIGRATIONS.unreleased.md(1 hunks)app/models/folder/Folder.scala(1 hunks)app/models/user/User.scala(1 hunks)conf/evolutions/128-replace-text-types.sql(1 hunks)conf/evolutions/reversions/128-replace-text-types.sql(1 hunks)tools/postgres/schema.sql(25 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/models/user/User.scala (1)
app/models/folder/Folder.scala (1) (1)
isEditable(273-286)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: circleci_build
🔇 Additional comments (9)
MIGRATIONS.unreleased.md (1)
14-14: Validates core purpose of the PR.The addition of migration
128-replace-text-types.sqlaligns with the PR objective to replace char/varchar types with TEXT in the database. This entry correctly documents the migration in the unreleased changes.conf/evolutions/reversions/128-replace-text-types.sql (1)
1-9: Good transaction handling and version control.The reversion script properly handles schema versioning with appropriate transaction boundaries. The script correctly:
- Verifies the current schema version is 128
- Explicitly declares no reversion (which makes sense for a type change)
- Updates the schema version back to 127
This approach is appropriate since reverting TEXT back to specific varchar/char types could potentially lead to data truncation.
app/models/folder/Folder.scala (1)
323-325: Improved query structure with EXISTS and table aliases.The SQL query has been refactored to use table aliases (
fp1andfp2) and anEXISTSclause rather than direct conditions. This approach:
- Improves query readability with explicit table aliases
- Uses the more efficient
EXISTSclause for checking conditions- Makes the exclusion logic more explicit and maintainable
This change is consistent with PostgreSQL best practices for complex queries.
app/models/user/User.scala (3)
173-174: Improved query efficiency with EXISTS pattern.Excellent change from using
INwith a subquery to usingEXISTSwith a correlated subquery, which is generally more efficient in PostgreSQL. The addition of table aliases (utr1andutr2) also improves code readability and maintenance.
176-176: Properly structured condition with EXISTS clause.The condition correctly uses the EXISTS pattern to check for users in teams managed by the requesting user. This matches the pattern used in other parts of the codebase like the
isEditablemethod in Folder.scala.
178-178: Consistent NOT EXISTS implementation.The NOT EXISTS version of the condition maintains the same structure as the positive case, ensuring consistency and readability.
tools/postgres/schema.sql (1)
18-24: Update Schema Version in Release InformationThe insertion of the new schema version (128) into the releaseInformation table properly reflects the migration’s intent. This check helps ensure that subsequent application logic will work with the updated schema.
conf/evolutions/128-replace-text-types.sql (2)
1-8: Initial Assertion and Rationale CommentsThe migration begins with an assertion that confirms the current schema version is 127 before any changes are applied. The following comments explain the strategy—performing changes in multiple transactions to avoid prolonged locks are both clear and well justified.
486-487: Final Schema Version Update ConfirmationThe final statement updates the releaseInformation table to set schemaVersion to 128, which neatly finalizes this evolution. Just ensure that all application components dependent on this version are verified after migration.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
conf/evolutions/128-replace-text-types.sql (2)
9-22: Annotations Table Transaction BlockThis transaction block for the
webknosos.annotationstable is well structured:
- It drops the associated view
- Alters each column’s data type to TEXT (with appropriate default value adjustments such as setting an empty string for
nameand initializing thetagsarray)- Recreates the view based on non-deleted records
- And finally commits the transaction.
A suggestion for improved idempotency: consider using the
IF EXISTSclause with each DROP VIEW statement (e.g.,-DROP VIEW webknosos.annotations_; +DROP VIEW IF EXISTS webknosos.annotations_; ```) so that the migration does not fail if the view is already absent. --- `30-483`: **Consistent Transaction Blocks for Column Type Updates** Every subsequent transaction block—from those updating tables such as `annotation_sharedTeams`, `annotation_contributors`, `annotation_mutexes`, `meshes`, `publications`, `datasets`, and many others—follows a consistent and clear pattern: - When applicable, related views are dropped before the alteration, then recreated after the changes. - Each table’s columns are individually altered to the TEXT (or TEXT[] for array columns) type, and default values are reset where needed. This modular transaction approach minimizes the impact of long locks and isolates potential failures to individual tables. **Recommendation:** For enhanced robustness, it is advisable to add `IF EXISTS` conditions in all DROP VIEW statements. For example: ```diff -DROP VIEW webknosos.some_view_; +DROP VIEW IF EXISTS webknosos.some_view_;This change can help ensure that the script behaves gracefully if a view is absent (for example, due to a partial migration or manual intervention).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
conf/evolutions/128-replace-text-types.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: circleci_build
🔇 Additional comments (4)
conf/evolutions/128-replace-text-types.sql (4)
1-2: Schema Version Assertion CheckThe initial PL/pgSQL block asserts that the current schema version is 127 before proceeding. This is a solid safety check to prevent accidental application of the migration on an unexpected schema version.
3-8: Clear Documentation on Migration StrategyThe introductory comments clearly describe the intent of replacing CHAR/VARCHAR with TEXT and explain why the migration is split into multiple transactions (to reduce long locks and allow partial completion). This provides useful context for future maintainers.
24-29: Annotation Layers Table UpdateThe transaction block for
webknosos.annotation_layerscleanly alters the_annotation,tracingId, andnamecolumns to TEXT. The use of START TRANSACTION and COMMIT ensures that these changes are atomic.
485-486: Schema Version Update ConfirmationThe final update sets the schema version to 128 in the
webknosos.releaseInformationtable, ensuring that the versioning reflects the new state of the database. This is a critical step in safely marking the completion of the migration.
|
@fm3 If this is approved, I would merge Monday morning. |
|
Did you have a look at the create domain option as mentioned above? Could this be a good fit here? |
|
I'm not sure what the purpose would be? Just to rename the column type? Or do you want to introduce a length constraint? |
|
The main purpose would be to enable us to see the objectIds at a glance, so yes, renaming. But the 24-character limit might also provide a useful sanity check. What do you think? |
I don't care about it either way |
|
Alright then I’ll have to decide! Please have a look if there may be significant downsides to this, and if not, implement it :) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
conf/evolutions/reversions/129-replace-text-types.sql (1)
1-10: Reversion Script Consistency Check
The script correctly wraps the reversion in a transaction and updates the schema version from 129 to 128. However, note the comment on line 5 stating “-- No reversion for this evolution”, which is misleading given that the script does perform a reversion by updating the schema version. Consider updating or removing the comment to avoid confusion.conf/evolutions/129-replace-text-types.sql (3)
14-27: Transactional Alterations for the Annotations Table
This transaction:
- Drops the dependent view,
- Alters several columns (e.g. _id, _dataset, _task, etc.) to the new ObjectId or TEXT type,
- Resets default values for columns such as name and tags, and
- Recreates the view filtering out deleted rows.
Splitting the changes into a dedicated transaction per table is a sound approach to minimize locking. One suggestion is to consider using “IF EXISTS” when dropping views for greater resilience if the view does not exist.
84-89: Transactional Alterations for Dataset Layers
The changes to thedataset_layerstable (altering_dataset,name, andmappings) are appropriately wrapped in a transaction. Converting themappingscolumn to TEXT[] (if its contents are already text) should be validated on existing data.
205-221: Transactional Alterations for Organizations and User Information Views
This block updates several columns in theorganizationstable (including converting_id, additional info, and logos) and rebuilds theuserInfosview joining withusers,organizations, andmultiUsers. It’s detailed and appears carefully executed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
MIGRATIONS.unreleased.md(1 hunks)conf/evolutions/129-replace-text-types.sql(1 hunks)conf/evolutions/reversions/129-replace-text-types.sql(1 hunks)tools/postgres/schema.sql(25 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- MIGRATIONS.unreleased.md
🔇 Additional comments (57)
conf/evolutions/129-replace-text-types.sql (49)
1-8: Initial Preconditions and Documentation Clarity
Lines 1–8 set a pre-condition (asserting the schema version equals 128) and document the migration strategy. This is a clear and proper safeguard before applying a multi-transaction schema change.
9-11: Domain Definition for ObjectId
The new domain for ObjectId is created with a check constraint enforcing a 24‑character hexadecimal format. This ensures that any column using this domain will contain valid MongoDB ObjectIds.
29-33: Transactional Alterations for Annotation Layers
The alterations to theannotation_layerstable (conversion of_annotation,tracingId, andnamecolumns) are straightforward. The transaction structure is consistent with the approach used elsewhere in the file.
35-48: Transactional Alterations for Shared, Contributors, and Mutex Tables
The blocks altering the_annotation,_team, and_usercolumns in tables such asannotation_sharedTeams,annotation_contributors, andannotation_mutexesfollow the same sensible pattern. Each transaction is isolated, reducing the risk of impact on other operations.
50-55: Transactional Alterations for Meshes and Their Dependent Views
Dropping and recreating the meshes view after altering column types (for_idand_annotation) is correct and follows the established practice.
57-63: Transactional Alterations for Publications
The script updates the_idcolumn and convertsimageUrlandtitleto TEXT, then re-establishes the view filtering out deleted records. This ensures consistency with the new schema.
65-82: Transactional Alterations for Datasets
Multiple columns in thedatasetstable (e.g.,_id,_dataStore,_organization,name,directoryName,status,sharingToken, andtags) are altered appropriately to TEXT or ObjectId where needed. The addition of default values (e.g. fornameandstatus) is good practice. Ensure that existing data conforms to these new types to avoid conversion issues.
90-94: Transactional Alterations for Dataset Layer Coordinate Transformations
The conversion of_dataset,layerName, andtypeto ObjectId and TEXT respectively is clear and consistent.
96-100: Transactional Alterations for Additional Axes in Dataset Layers
The script changes the column types for_dataset,layerName, andnameindataset_layer_additionalAxesas required. This transaction appears correct.
102-112: Transactional Alterations for Allowed Teams and Mags
Transactions altering columns indataset_allowedTeamsanddataset_magsare straightforward. These modifications maintain referential integrity and update the data types to the new standards.
112-122: Transactional Alterations for Last Used Times and Thumbnails
The changes adjust columns indataset_lastUsedTimesanddataset_thumbnailseffectively. Make certain that the new TEXT types accommodate all previously stored values.
124-132: Transactional Alterations for DataStores
Altering multiple columns (e.g.,name,url,publicUrl,key, andonlyAllowedOrganization) indataStoresand then recreating the view is done correctly. The text pattern checks remain in place.
134-141: Transactional Alterations for TracingStores
The script converts relevant columns intracingStoresto TEXT and re-establishes the view accordingly. Everything is consistent with the migration strategy.
143-151: Transactional Alterations for Projects
The modifications in theprojectstable include converting_id,_organization,_team,_owner, andnameto their new types and ensuring a non-deleted view is re-created. This is implemented correctly.
153-160: Transactional Alterations for Scripts
The alterations in thescriptstable (changing_id,_owner,name, andgist) and the subsequent view creation are proper. The check for a valid URL ingistis maintained.
162-169: Transactional Alterations for Task Types
Altering task type columns (such as_id,_organization,_team, andsummary) and recreating the view follows the same robust pattern.
171-180: Transactional Alterations for Tasks
The changes to thetaskstable are comprehensive, updating several foreign key columns and text fields (includingneededExperience_domainandcreationInfo). Rebuilding the view after these conversions helps ensure data consistency.
182-189: Transactional Alterations for Experience Domains and Teams
The modifications forexperienceDomainsandteams(with an update to views such asteams_andorganizationTeams) are thorough. Dropping/recreating views to apply the new types is correctly handled.
197-203: Transactional Alterations for Timespans
The conversion of timespan columns is straightforward. The view re-creation at the end of the transaction ensures that only active records are visible.
232-238: Transactional Alterations for Organization Used Storage
The column type updates here are clear and should maintain data fidelity.
240-260: Transactional Alterations for Users and Rebuilding User Infos
The script converts user-related columns to ObjectId and TEXT where applicable, and then recreates bothusers_anduserInfosviews. Keep an eye on the consistency of these view definitions, as they are re-created multiple times throughout the migration file.
262-266: Transactional Alterations for User Team Roles
The update to theuser_team_rolestable is simple and conforms with the new types.
267-271: Transactional Alterations for User Experiences
The conversion of_userand thedomaincolumn inuser_experiencesis handled well.
272-281: Transactional Alterations for User Dataset and Layer Configurations
The changes here update ObjectId columns inuser_datasetConfigurationsanduser_datasetLayerConfigurationsand convertlayerNameto TEXT accordingly.
283-301: Transactional Alterations for MultiUsers and Rebuilding User Infos Again
The conversion of_id,passwordInfo_password, and_lastLoggedInIdentityinmultiUsersis executed correctly, and the viewsmultiUsers_anduserInfosare re-established accurately.
303-308: Transactional Alterations for Tokens
The type conversion for tokens, especially transformingloginInfo_providerKeyto TEXT, is done properly and the view is re-created.
310-315: Transactional Alterations for Maintenances
The updates for themaintenancestable, including changing_idand_usertypes, are straightforward.
317-327: Transactional Alterations for Workers
The changes involve converting worker-related columns to ObjectId or TEXT, as well as setting a default for the worker’s name and key. The conversion of the supported job commands array is also correctly executed.
329-340: Transactional Alterations for Jobs
The jobs table sees multiple column type changes, including foreign key columns and text fields. This transaction follows the established pattern and re-creates the corresponding view.
342-347: Transactional Alterations for Invites
The alterations in theinvitestable, particularly converting_idand_organizationto ObjectId or TEXT, are handled and the view is maintained.
349-355: Transactional Alterations for Annotation Private Links
The conversion of_id,_annotation, andaccessTokenin theannotation_privateLinkstable is straightforward, and rebuilding the view is correct.
357-361: Transactional Alterations for ShortLinks
The changes update columns to ObjectId and TEXT as needed. This transaction is succinct and appears correct.
363-372: Transactional Alterations for Credentials
The credentials table now uses ObjectId for_id, and columns such asname,identifier, andsecrethave been transitioned to TEXT. The re-creation of the view ensures consistency.
374-378: Transactional Alterations for Folders
Updating the_idcolumn infoldersand recreating the view is handled correctly.
380-383: Transactional Alterations for Folder Paths
The conversion for_ancestorand_descendantto ObjectId is straightforward and does not introduce any complexity.
385-388: Transactional Alterations for Folder Allowed Teams
The update to columns infolder_allowedTeamsis implemented correctly.
390-394: Transactional Alterations for Email Verification Keys
The conversion of_id,_multiUserin theemailVerificationKeystable is proper and completes the transaction successfully.
396-406: Transactional Alterations for AI Models
The modifications in theaiModelstable (changing_id,_organization,_dataStore,_user,_trainingJob,name, andcomment) are consistent with the overall migration. The view is re-created accordingly.
408-412: Transactional Alterations for AI Model Training Annotations
The simple conversion of_aiModeland_annotationto ObjectId in the training annotations table looks correct.
413-424: Transactional Alterations for AI Inferences
Multiple columns are altered (including various foreign keys and text fields such asnewSegmentationLayerNameandmaskAnnotationLayerName). The re-creation of the view ensures that only active (non-deleted) records are visible.
426-432: Transactional Alterations for Voxelytics Artifacts
The changes in thevoxelytics_artifactstable (altering_id,_task,name,path, andversion) are correctly implemented.
434-444: Transactional Alterations for Voxelytics Runs
The updates to thevoxelytics_runstable include converting several identifier and text columns, along with ensuring the JSONB column for workflow configuration is still properly constrained.
446-451: Transactional Alterations for Voxelytics Tasks
The modifications tovoxelytics_tasks, especially changing ObjectId and TEXT fields, are correctly processed.
453-458: Transactional Alterations for Voxelytics Chunks
The conversion of identifiers and text fields (e.g.executionIdandchunkName) in thevoxelytics_chunkstable is well handled.
460-464: Transactional Alterations for Voxelytics Workflows
The updates to workflows (converting_organization,hash, andnameto TEXT) and wrapping in a transaction are consistent with the approach used throughout the script.
466-469: Transactional Alterations for Voxelytics RunHeartbeat Events and Chunk Profiling Events
These transactions convert the necessary foreign key columns (e.g._runin heartbeat events and_chunkin profiling events) to ObjectId. They are succinct and correct.
474-480: Transactional Alterations for Voxelytics Artifact File Checksum Events
The script updates several columns to TEXT (e.g.path,resolvedPath,checksumMethod, andchecksum) and converts_artifactto ObjectId. The transaction appears correct.
482-488: Transactional Alterations for Analytics Events
The conversion in theanalyticsEventstable (columns_id,eventType,_user,_organization, andwebknossosUri) is handled properly within a transaction.
490-490: Final Schema Version Update
Updating thereleaseInformationtable to reflect the new schema version (129) finalizes the migration. This is essential for tracking and coordinating future schema changes.tools/postgres/schema.sql (8)
18-21: Domain Definition Consistency in Schema
The domainObjectIdis defined at lines 18–21 with the appropriate constraint. This definition is consistent with the changes in the evolutions script and ensures all columns using ObjectId will be validated.
27-28: Schema Version Insertion Update
Line 27 inserts the new schema version (129) intoreleaseInformation. This aligns with the migration changes.
31-58: Table Definitions Using ObjectId and TEXT
Many table definitions (e.g., forannotations,datasets,annotation_layers, etc.) now use the newObjectIddomain and update various character columns toTEXT. The modifications appear comprehensive and consistent with the migration objectives.
140-155: Dataset Table Column Type Updates
Changes in thedatasetstable—including the conversion of_id,_dataStore,name,directoryName,status,sharingToken, andtags—are consistent with the PR objective of replacing fixed-length types with TEXT.
256-278: Task Types and Tasks Tables Adjustments
The adjustments in the definitions fortaskTypesandtasks, where identifier columns and text-based columns are updated, are correct and reflect the intended schema improvements.
826-833: User Dataset and Layer Configurations, and MultiUsers Views
Columns in various user-related tables have been updated to use the new ObjectId and TEXT definitions. The appropriate constraints (e.g., JSONB type checks) remain in place, promoting consistency across the schema.
843-851: Foreign Key Constraints and Referential Integrity
The foreign key constraints throughout the schema (e.g., inannotations,datasets,teams, etc.) have been updated to reference the new column types. Verify that all table names and referenced domains (e.g.,ObjectId) are consistent; there is a minor potential risk with case sensitivity in table names (e.g.datastoresvs.dataStores) that may need double-checking in your deployment environment.
893-908: Helper Functions and Triggers
The functions (e.g.,countsAsTaskInstance,onUpdateTask, etc.) and associated triggers remain unchanged. They function based on the data in the tables and are unaffected by the type changes since the new types are compatible with the previous representations.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
tools/postgres/schema.sql (1)
570-574: 🛠️ Refactor suggestion
⚠️ Potential issueAIModel_organizations – Inconsistent Types Detected
Thewebknosos.aiModel_organizationstable still usesCHAR(24)for_aiModelandVARCHAR(256)for_organization(lines 571–572), which is inconsistent with the other schema changes that have replacedchar/varcharwithTEXT. For uniformity (and to avoid potential foreign key mismatches withwebknosos.aiModels._id, which is nowTEXT), consider updating these columns toTEXTwith appropriate check constraints. For example:- _aiModel CHAR(24) NOT NULL, - _organization VARCHAR(256) NOT NULL, + _aiModel TEXT CHECK (_aiModel ~ '^[0-9a-f]{24}$') NOT NULL, + _organization TEXT NOT NULL,
🧹 Nitpick comments (1)
tools/postgres/schema.sql (1)
1-960: General Consistency & Performance Considerations
This migration makes a comprehensive change to replaceCHAR(n)andVARCHAR(n)withTEXTon many tables. Overall, the approach is consistent and simplifies identifier handling by relying on check constraints where a fixed format is required. Please verify that all foreign key relationships remain intact and consider any potential performance implications on large tables or indexes due to the removal of fixed-length types. Extensive testing (ideally on a production-like dump) is recommended before final deployment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
conf/evolutions/129-replace-text-types.sql(1 hunks)tools/postgres/schema.sql(25 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- conf/evolutions/129-replace-text-types.sql
🔇 Additional comments (54)
tools/postgres/schema.sql (54)
23-23: Schema Version Bump
The migration now explicitly inserts a schema version of 129 (line 23), which aligns with the new migration script [129-replace-text-types.sql]. Please verify that all downstream tools and documentation (e.g. the migration guide) are updated accordingly.
31-43: Annotations Table – Identifier Conversion
All identifier columns in thewebknossos.annotationstable (lines 31–36, 39, 43) have been converted to useTEXTwith a check constraint enforcing a 24-character hexadecimal format. This is consistent with the PR objective. Please double-check that the regex ('^[0-9a-f]{24}$') correctly validates the intended format in all use cases.
56-60: Annotation Layers – Consistent TEXT Usage
Thewebknossos.annotation_layerstable now defines its identifier columns (e.g._annotationon line 56 andtracingIdon line 57) and thenamecolumn (line 59) asTEXT(with appropriate check constraints). This ensures consistency with the new TEXT-based schema.
67-68: Annotation Shared Teams – TEXT Conversion
The columns_annotationand_teamin thewebknossos.annotation_sharedTeamstable (lines 67–68) have been updated toTEXTwith the intended check constraints. This meets the migration objective.
73-74: Annotation Contributors – Identifier Update
The conversion of_annotationand_userfields toTEXTin thewebknossos.annotation_contributorstable (lines 73–74) is straightforward and maintains consistency with related tables.
79-80: Annotation Mutexes – Identifier Consistency
The fields inwebknossos.annotation_mutexes(lines 79–80) are now declared asTEXTwith check constraints. Please ensure that any application logic relying on these values is fully compatible with the new type.
85-86: Meshes Table – TEXT Identification
Both_idand_annotationin thewebknossos.meshestable (lines 85–86) have been converted toTEXTwith a hexadecimal check. This change is in line with the objective to standardize identifier types.
95-98: Publications Table – Updated Identifier & Descriptive Fields
In thewebknossos.publicationstable, the_idfield (line 95) and other textual columns (lines 97–98) now useTEXTalong with check constraints. Confirm that this change does not affect any application logic that might have assumed fixed-length values.
105-124: Datasets Table – Comprehensive TEXT Conversion
Thewebknossos.datasetstable (lines 105–124) now defines several identifier columns (such as_id,_dataStore,_organization,_publication,_uploader, and_folder) asTEXTalong with regex-based check constraints. Additionally, descriptive fields likename,status, andsharingTokenare updated. Please ensure that the removal of existing length limits will not have unintended side effects and that all default values and constraints still meet business requirements.
139-146: Dataset Layers – Identifier and JSON Checks
For thewebknossos.dataset_layerstable (lines 139–146), identifier fields and related columns are uniformly converted toTEXTwith constraints, while JSON configuration fields retain their checks. This is a coherent update in line with the PR objectives.
154-157: Dataset Layer Coordinate Transformations – TEXT Consistency
The conversion of_dataset,layerName, andtypein thewebknossos.dataset_layer_coordinateTransformationstable (lines 154–157) toTEXTmaintains consistency across the schema.
163-168: Dataset Layer Additional Axes – Updated Identifiers
Inwebknossos.dataset_layer_additionalAxes(lines 163–168), the identifier fields are nowTEXTwith the expected check constraints. The numerical bounds remain unchanged.
172-173: Dataset Allowed Teams – Uniform TEXT Types
The_datasetand_teamcolumns inwebknossos.dataset_allowedTeams(lines 172–173) have been successfully updated toTEXTwith validation constraints.
178-179: Dataset Mags – Identifier Conversion
The changes in thewebknossos.dataset_magstable (lines 178–179) converting the_datasetfield toTEXTare in line with the overall schema update.
188-189: Dataset LastUsedTimes – TEXT for Identifiers
The identifier columns in thewebknossos.dataset_lastUsedTimestable (lines 188–189) are now consistently defined asTEXTwith check constraints.
194-200: Dataset Thumbnails – TEXT and BYTEA Fields
The update inwebknossos.dataset_thumbnails(lines 194–200) converts identifier columns toTEXTwhile maintaining the integrity of image storage viaBYTEA. Verify that the check constraints and unique conditions remain effective.
209-216: DataStores – Regex-Validated TEXT Fields
In thewebknossos.dataStorestable (lines 209–216), columns likename,url, andpublicUrlhave been switched toTEXTwith corresponding regex validations. Please confirm that these patterns (e.g. for URLs) are comprehensive enough to reject malformed entries.
221-224: TracingStores – Standardized TEXT Declarations
Thewebknossos.tracingStorestable (lines 221–224) now fully employsTEXTfor its identifier and URL fields while preserving unique constraints and check validations.
229-233: Projects Table – Identifier & Foreign Key Consistency
Thewebknossos.projectstable (lines 229–233) features updated identifier columns and related foreign key candidate columns asTEXTwith check constraints. Verify that the uniqueness and non-deletion requirements continue to be enforced in the application layer.
243-246: Scripts Table – Updated TEXT Fields with Regex
In thewebknossos.scriptstable (lines 243–246), the_id,_owner, andnamecolumns have been converted toTEXTwith appropriate regex constraints. This simplifies string handling for script metadata.
255-263: TaskTypes Table – TEXT Conversion and Defaults
Thewebknosos.taskTypestable (lines 255–263) now usesTEXTfor identifier fields (including_id,_organization, and_team) along with check constraints. Be sure that any numeric settings or defaults (e.g. branch points, min/max restrictions) remain correctly handled.
277-289: Tasks Table – Maintaining Referential Integrity
Key identifier fields in thewebknosos.taskstable (lines 277–280, 289) have been updated toTEXTto match the new schema. Confirm that related foreign key references in other tables (e.g. projects, scripts) still resolve correctly after conversion.
296-304: Administrative and Organizational Tables – TEXT Updates
The tables forexperienceDomains,teams, and part oforganizations(lines 296–304) now employTEXTfor their identifier columns with corresponding check constraints. For example, inwebknossos.organizations(lines 324–330),_id_oldand_idare defined with TEXT (with_id_oldincluding a check). Ensure that any legacy code handling the old ID type is updated accordingly.
345-349: Organization Used Storage – Consistent TEXT Identifiers
Thewebknosos.organization_usedStoragetable (lines 345–349) has its identifier columns (e.g._organization,_dataStore,_dataset) converted toTEXT. This change should help maintain consistent foreign key relationships throughout the schema.
357-359: Users Table – TEXT for User IDs and Related Fields
In thewebknosos.userstable (lines 357–359 and line 369), key fields such as_id,_multiUser, andlastTaskTypeIdare now defined asTEXTwith a check constraint ensuring a 24-character hexadecimal format. Confirm that authentication and user-lookup functionality remains intact.Also applies to: 369-369
377-378: User Team Roles – Identifier Consistency
The_userand_teamfields inwebknosos.user_team_roles(lines 377–378) are now declared asTEXTwith check constraints, ensuring they are consistent with the updated user and team identifiers.
384-385: User Experiences – TEXT Conversion
Thewebknosos.user_experiencestable (lines 384–385) correctly converts the_userfield toTEXT, aiding consistency across the schema.
390-400: User Dataset Configurations – Updated Identifier Types
The identifier columns in bothwebknosos.user_datasetConfigurations(lines 390–392) andwebknosos.user_datasetLayerConfigurations(lines 398–400) are now defined asTEXTwith precise check constraints. This change should preserve all necessary relationships between users and datasets.
410-411: MultiUsers Table – TEXT and Email Validation
Thewebknosos.multiUserstable (lines 410–411 and 418) now usesTEXTfor its identifier fields and applies a regex check onAlso applies to: 418-418
428-428: Tokens Table – Identifier Updated to TEXT
The_idfield in thewebknosos.tokenstable (line 428) is correctly updated toTEXTwith its check constraint.
441-442: Maintenances Table – Consistent TEXT Types
Identifier columns in thewebknosos.maintenancestable (lines 441–442) now useTEXT, which should help the foreign key relationships remain consistent.
451-457: Workers Table – TEXT for Identifier Fields
Thewebknosos.workerstable (lines 451–457) converts its_idand_dataStorefields toTEXTwith check constraints. Verify that the default values (such as forname) are still appropriate.
467-475: Jobs Table – TEXT Conversion and FK Integrity
Key columns inwebknosos.jobs(lines 467–475) have been updated toTEXT. Please ensure that every foreign key (e.g._owner,_dataStore,_worker) references the corresponding updated TEXT fields in their parent tables.
487-489: Invites Table – Updated TEXT Fields
The conversion of_idand_organizationin thewebknosos.invitestable (lines 487–489) toTEXTis straightforward. Confirm that token handling logic remains unaffected.
496-498: Annotation PrivateLinks – Identifier Consistency
The_idand_annotationfields inwebknosos.annotation_privateLinks(lines 496–498) are now consistently defined asTEXTwith the hexadecimal check.
505-506: ShortLinks Table – TEXT for Keys
Thewebknosos.shortLinkstable (lines 505–506) correctly updates its identifier andkeyfields toTEXT.
512-518: Credentials Table – Secure TEXT Types
Inwebknosos.credentials(lines 512–518), identifier and related fields are converted toTEXT. Please check that any sensitive information (e.g.secret) is still stored and managed securely.
524-529: Folders Table – TEXT Conversion
The_idfield inwebknosos.folders(line 524) now usesTEXTwith a check to prevent invalid characters (line 525). This is a clear improvement for consistency.
531-536: Folder Paths – TEXT for Hierarchical IDs
The columns_ancestorand_descendantin thewebknosos.folder_pathstable (lines 531–533) are updated to useTEXTwith the proper check constraints. This maintains the integrity of the folder hierarchy.
538-539: Folder Allowed Teams – Uniform TEXT Types
The_folderand_teamcolumns inwebknosos.folder_allowedTeams(lines 538–539) are now uniformly defined asTEXT, in keeping with the overall changes.
544-548: Email Verification Keys – TEXT and Email Format
Thewebknosos.emailVerificationKeystable (lines 544–548) updates its identifiers and email-related fields toTEXT. Make sure the email validation regex is comprehensive enough for real-world addresses.
556-560: AIModels Table – Comprehensive TEXT Conversion
Thewebknosos.aiModelstable (lines 556–560) now declares its identifier fields and foreign key dependencies asTEXTwith check constraints. This should simplify identifier handling across AI model records.
576-577: AIModel Training Annotations – Identifier Update
Thewebknosos.aiModel_trainingAnnotationstable (lines 576–577) correctly converts its identifier columns toTEXTwith the necessary check constraints.
582-591: AIInferences Table – TEXT and Referential Integrity
Thewebknosos.aiInferencestable (lines 582–591) now usesTEXTfor all key identifiers while also defining new columns likenewSegmentationLayerNameandmaskAnnotationLayerNameasTEXT. Verify that the updated foreign keys (such as referencing_aiModel) still resolve correctly after conversion.
600-607: Voxelytics Artifacts – Uniform TEXT Identifiers
Inwebknosos.voxelytics_artifacts(lines 600–607), identifier columns are updated toTEXT, and the unique constraints remain in place. Ensure that file size and inode data are still handled as intended.
615-622: Voxelytics Runs – TEXT Conversions
Thewebknosos.voxelytics_runstable (lines 615–622) now declares its identifier fields asTEXTconsistently while preserving foreign key relationships (e.g._organization,_user). Confirm that the unique constraint on (_organization, name) is still optimal with these changes.
634-642: Voxelytics Tasks – Ensuring Consistency
The updatedwebknosos.voxelytics_taskstable (lines 634–642) converts its identifiers and configuration toTEXTand JSONB, respectively. Check that the reference to_runremains correctly defined after the type change.
647-656: Voxelytics Chunks – TEXT for Identifiers
Identifier fields in thewebknosos.voxelytics_chunkstable (lines 647–656) are now defined asTEXT, ensuring consistency with the rest of the schema.
659-664: Voxelytics Workflows – Text-Based Identifiers
The tablewebknosos.voxelytics_workflows(lines 659–664) now usesTEXTfor both_organizationandhash. This change should have little impact provided that join conditions are verified.
667-669: Voxelytics RunHeartbeatEvents – Updated TEXT FK
The_runfield inwebknosos.voxelytics_runHeartbeatEvents(lines 667–669) is nowTEXTand will correctly reference the corresponding changed field invoxelytics_runs.
672-680: Voxelytics Chunk Profiling Events – Identifier Updates
Thewebknosos.voxelytics_chunkProfilingEventstable (lines 672–680) properly converts the_chunkfield toTEXT. Please verify that the numeric fields (e.g. memory, cpu values) remain correctly typed.
683-692: Voxelytics Artifact File Checksum Events – TEXT Consistency
Inwebknosos.voxelytics_artifactFileChecksumEvents(lines 683–692), identifier columns are updated toTEXT, and checksum-related fields are maintained asTEXT. This should ensure that file-check algorithms are unaffected.
695-701: Analytics Events – Standardized TEXT Usage
Thewebknosos.analyticsEventstable (lines 695–701) now usesTEXTfor its identifier and event type fields. The check constraint oneventPropertiesremains in place. Confirm that reporting and logging operations handle these TEXT updates without issue.
889-960: Triggers and Functions – Verification of Implicit Type Assumptions
The procedural code (lines 889–960) including functions and triggers (e.g.onUpdateTask(),onInsertAnnotation(), etc.) remains largely unaffected by the underlying type changes. It is recommended to run integration tests to ensure that no implicit assumptions about column types (such as expecting a fixed-length string) have been violated.
frcroth
force-pushed
the
postgres-best-practices
branch
from
78dc8c8 to
9efc9c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
tools/postgres/schema.sql (1)
571-575: 🛠️ Refactor suggestion
⚠️ Potential issueInconsistency: aiModel_organizations Table Column Types
In the definition of theaiModel_organizationstable (lines 571–575), the column_aiModelis still defined asCHAR(24)and_organizationasVARCHAR(256). Since the PR objective is to replace all instances ofchar(n)andvarchar(n)withTEXT, these should be updated accordingly.Below is a suggested diff change for this segment:
-CREATE TABLE webknossos.aiModel_organizations( - _aiModel CHAR(24) NOT NULL, - _organization VARCHAR(256) NOT NULL, - PRIMARY KEY(_aiModel, _organization) -); +CREATE TABLE webknossos.aiModel_organizations( + _aiModel TEXT CONSTRAINT _aiModel_objectId CHECK (_aiModel ~ '^[0-9a-f]{24}$') NOT NULL, + _organization TEXT NOT NULL, + PRIMARY KEY(_aiModel, _organization) +);Please verify that these changes align with the intended design for this table.
🧹 Nitpick comments (2)
conf/evolutions/129-replace-text-types.sql (2)
11-31: Annotations Table Migration Block – Structure and Consistency
This transaction block for the “annotations” table is well structured. It drops the dependent view, alters columns to TEXT, and adds the appropriate check constraints (e.g. enforcing that ObjectIds match the 24-character lowercase hex format). One suggestion: to improve idempotency and avoid errors if the view does not exist, you might consider using “DROP VIEW IF EXISTS” rather than “DROP VIEW”.
53-67: Other Table Migration Blocks (mutexes, meshes, publications, datasets, etc.)
Throughout the remainder of the file the pattern is consistent: each table is handled in its own transaction to reduce lock duration, and the views dropped before alterations are recreated afterward.
A few points to consider:
- Drop View Robustness: In several transaction blocks (e.g. for “meshes”, “publications”, etc.), using “DROP VIEW IF EXISTS” could prevent errors if a view is unexpectedly missing.
- Regex Strictness: The constraints use the regex
'^[0-9a-f]{24}$'which only accepts lowercase hex. If there is any possibility for uppercase letters, you might want to consider a case‑insensitive operator (e.g.~*) or adjust the regex accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
conf/evolutions/129-replace-text-types.sql(1 hunks)tools/postgres/schema.sql(25 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: circleci_build
🔇 Additional comments (9)
conf/evolutions/129-replace-text-types.sql (4)
1-1: Schema Version Assertion Verification
The initial DO block (line 1) correctly asserts that the current schema version is 128 before starting the migration.
32-37: Annotation Layers Transaction Block
The transaction block for the “annotation_layers” table (lines 32–37) follows the same pattern as the annotations block. The check constraint on the “name” column (using a case‐insensitive match with ~*) helps validate the allowed characters.
39-44: Shared Teams and Contributors Blocks
The blocks altering the “annotation_sharedTeams” (lines 39–44) and “annotation_contributors” (lines 46–51) tables consistently update columns to TEXT with the necessary object ID check constraints.
589-590: Final Schema Version Update
The script concludes by updating theschemaVersionto 129 (line 589). This final update is appropriate and clearly marks the completion of the migration.tools/postgres/schema.sql (5)
1-8: Schema and Type Definitions – Initialization
The initial part of the file correctly drops (if existing) and recreates the schema “webknossos” and defines the composite types (VECTOR3 and BOUNDING_BOX). The definitions appear standard.
19-26: Release Information Table and Version Update
The transaction block (lines 19–26) creates thereleaseInformationtable and inserts the new schema version (129) as required by the migration. This is clear and aligned with the overall objective.
28-53: Annotations Table Definition in Schema
The definition of theannotationstable (lines 31–52) correctly uses TEXT for identifier fields and applies the check constraints enforcing the ObjectId pattern. Default values and data types for other fields (e.g. description, tags) appear appropriate.
577-942: Remaining Table, View, and Constraint Definitions
The remainder of the schema file continues to update tables by using TEXT for identifier fields along with appropriate check constraints. All other changes (views, indexes, foreign keys, functions, and trigger definitions) appear consistent with the new schema design.
A couple of minor notes:
- Ensure that any unquoted table names (e.g., in foreign key references such as
REFERENCES webknossos.datastores(name)) are correctly resolved by PostgreSQL (they will be folded to lower-case) and match the created table names.- The trigger functions (e.g.,
onUpdateTask,onInsertAnnotation, etc.) continue to work as expected based on the new data types.
890-909: Trigger and Function Definitions
The functions and triggers defined in the latter part (lines 890–909) are well formed. In particular, the functioncountsAsTaskInstanceand the associated triggers ensure that the changes in annotation state correctly update the pending instance counts on tasks.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
conf/evolutions/130-replace-text-types.sql (4)
32-58: Conversion of Related Tables – Annotation Layers, SharedTeams, Contributors, and MutexesA series of transactions (lines 32–58) convert identifier columns for the tables supporting annotations (e.g. in annotation_layers, annotation_sharedTeams, annotation_contributors, and annotation_mutexes). Each block applies the same pattern:
- Alter the data type to TEXT
- Re‑add the check constraint ensuring the proper 24‑hex format
This consistent approach is good. Verify that any application code addressing these fields now interprets them as variable‑length strings rather than fixed‑length.
Consider adding a brief inline comment in one block noting that the same pattern is repeated across similar tables.
142-148: Conversion of Dataset ThumbnailsThis block (lines 142–148) alters the _dataset, dataLayerName, mappingName, and mimetype columns to TEXT in the dataset_thumbnails table. Be sure that the removal of any length restrictions does not affect application logic (for instance, where string size may have been assumed).
A brief comment explaining that “empty string” defaults for certain text columns are maintained could improve clarity.
337-357: Conversion of MultiUsers and Recreation of UserInfos (Third Instance)The multiUsers table (lines 337–357) is updated: the _id, email, passwordInfo_password, and lastLoggedInIdentity columns are now TEXT. The corresponding recreation of views (multiUsers and userInfos) ensures that the migration remains coherent.
Consider a centralized test to verify that all three instances of the userInfos view produce identical results.
605-637: Updated Free Credits Function and Final Schema Version UpdateFinally, lines 605–637 drop and recreate the function hand_out_monthly_free_credits so it now treats its organization_id as TEXT. The function iterates through organizations, checking for an existing transaction based on DATE_TRUNC and inserting free credits if none exists. The script concludes by updating the schema version in the releaseInformation table.
If there is any concern about the performance of the loop in large organizations tables, a review of indexing on the expiration_date column (or rewriting the query) could be considered.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
MIGRATIONS.unreleased.md(1 hunks)conf/evolutions/130-replace-text-types.sql(1 hunks)conf/evolutions/reversions/130-replace-text-types.sql(1 hunks)tools/postgres/schema.sql(28 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- MIGRATIONS.unreleased.md
🔇 Additional comments (53)
conf/evolutions/reversions/130-replace-text-types.sql (1)
1-10: Schema Reversion Transaction and Assertion CheckThe reversion script correctly begins a transaction (line 1) and uses a PL/pgSQL DO block (line 3) to assert that the current schema version is 130 before downgrading it. Updating the version (line 7) and committing (line 9) guarantee that the system can roll back safely if the precondition isn’t met.
conf/evolutions/130-replace-text-types.sql (48)
1-10: Initial Schema Version Assertion and Documentation CommentsThe very start of the migration file (lines 1–10) asserts that the prior schema version is 129. The introductory comments (lines 3–8) explain the rationale for splitting the work into multiple transactions. This documentation is useful for later maintainers and helps justify the design choice.
11-30: Conversion of the Annotations Table (Lines 11–30)This transaction drops the old view for annotations, converts key identifier columns (_id, _dataset, _task, _team, _user, _publication) to TEXT, re-adds the check constraints to validate hexadecimal ObjectId values, and resets defaults (for example, for “name” and “tags”). Recreating the view afterward ensures that consumers of the data get only non‑deleted rows.
60-76: Conversion of Meshes and Publications TablesHere the script drops the old views (for meshes and publications), changes the identifier columns to TEXT (and—as needed—other columns like imageUrl or title), and then recreates the views filtering out deleted rows. The changes appear correct and in line with the overall migration objectives.
78-99: Conversion of the Datasets TableA longer transaction (lines 78–99) converts many columns in the datasets table—from the primary key and various foreign key columns (_id, _dataStore, _organization, _publication, etc.) to TEXT. Default values and array defaults (for tags) are also re‑defined. Because this table is central to many relationships, please double‑check that all foreign key dependencies (both in the schema and application code) are updated accordingly.
101-120: Conversion of Dataset Layers, Coordinate Transformations, and Additional AxesTransactions for tables such as dataset_layers (lines 101–107), dataset_layer_coordinateTransformations (lines 108–113), and dataset_layer_additionalAxes (lines 115–120) follow the same conversion pattern. The changes are mechanical and consistent.
122-127: Conversion of Dataset Allowed TeamsThe transaction in lines 122–127 alters _dataset and _team columns to TEXT in the dataset_allowedTeams table and re‑applies the check constraints.
129-134: Conversion of Dataset MagsLines 129–134 update the _dataset and dataLayerName columns in the dataset_mags table. Note that every alteration here diligently adds back the check constraint pattern for object IDs.
135-140: Conversion of Dataset Last Used TimesThe transaction (lines 135–140) converts the _dataset and _user columns to TEXT and re‑applies the associated check constraints.
150-167: Conversion of DataStores and TracingStoresThe transactions in lines 150–158 and 160–167 update the dataStores and tracingStores tables respectively. They convert key columns (name, url, publicUrl, key, etc.) to TEXT and re‑apply constraints (including regex checks ensuring proper URL formats) and then recreate the associated views.
169-191: Conversion of Projects and ScriptsThe transactions (lines 169–180 and 182–191) update projects and scripts. In both cases, identifier and foreign key columns are converted to TEXT and check constraints are re‑added. Re‑creating views afterward (projects_ and scripts_) helps in maintaining the integrity of query results.
193-202: Conversion of TaskTypesLines 193–202 alter the taskTypes table: converting _id, _organization, _team, and summary to TEXT while keeping suitable constraints.
204-217: Conversion of TasksIn this transaction (lines 204–217), key columns in the tasks table (including _id, _project, _script, taskType, neededExperience_domain, and creationInfo) are changed to TEXT and constraints re‑applied. Re‑creating the tasks view is also handled.
219-222: Conversion of Experience DomainsThe conversion of the experienceDomains table (lines 219–222) is straightforward; both the domain and _organization columns are now defined as TEXT.
224-233: Conversion of Teams and Associated ViewsIn this block (lines 224–233), the teams table is updated (using TEXT for _id, organization, and name) and both views (teams and organizationTeams) are recreated.
235-244: Conversion of TimespansThe transaction for timespans (lines 235–244) alters _id, _user, and _annotation to TEXT with the expected check constraints.
246-274: Conversion of Organizations and Recreation of UserInfosLines 246–274 update both the organizations table (including columns like _id_old, _id, additionalInformation, logoUrl, name, _rootFolder, and newUserMailingList) and recreate the userInfos view. Notice that default values are re‑applied where needed. With such core tables involved, please run full regression tests on any functionality that depends on organization identifiers.
275-282: Conversion of Organization Used StorageThis transaction (lines 275–282) alters columns in the organization_usedStorage table to TEXT.
284-308: Conversion of Users and Recreation of UserInfos (Second Instance)The block in lines 284–308 converts the users table columns (_id, _multiUser, organization, firstName, lastName, lastTaskTypeId) to TEXT and re‑applies constraints. The recreation of the users and userInfos views follows, ensuring consistency with the organizations changes.
309-315: Conversion of User Team RolesThis short transaction (lines 309–315) converts _user and _team columns in user_team_roles to TEXT and re‑adds check constraints.
316-320: Conversion of User ExperiencesThe change (lines 316–320) in the user_experiences table (for _user and domain) is correctly implemented.
322-327: Conversion of User Dataset ConfigurationsIn this transaction (lines 322–327), the _user and _dataset columns receive the TEXT type along with their check constraints.
329-336: Conversion of User Dataset Layer ConfigurationsChanging _user, _dataset, and layerName to TEXT in the user_datasetLayerConfigurations table (lines 329–336) is handled consistently with the other conversions.
359-366: Conversion of TokensThis block (lines 359–366) changes the _id column and loginInfo_providerKey for tokens to TEXT.
367-374: Conversion of MaintenancesThe alterations for the maintenances table (lines 367–374) are straightforward and consistent.
376-388: Conversion of WorkersThis transaction (lines 376–388) converts key columns in the workers table (_id, _dataStore, name, key, supportedJobCommands) to TEXT. Notice that a default value (“Unnamed Worker”) is maintained for the name.
389-403: Conversion of JobsLines 389–403 update jobs by converting columns (_id, _owner, _dataStore, command, _worker, _voxelytics_workflowHash, latestRunId, returnValue) to TEXT. Re‑adding constraints for _id, _owner, _worker helps maintain referential integrity.
405-411: Conversion of InvitesThe invites table is altered (lines 405–411) to convert _id and _organization to TEXT.
413-421: Conversion of Annotation PrivateLinksIn this transaction (lines 413–421) the _id, _annotation, and accessToken fields are converted to TEXT and constraints are re‑applied before the view is recreated.
423-428: Conversion of ShortLinksThe shortLinks table (lines 423–428) has its _id, key, and longLink columns converted to TEXT, which is implemented correctly.
430-441: Conversion of CredentialsThis block (lines 430–441) guarantees that credentials have their _id, name, identifier, secret, _user, and _organization defined as TEXT.
443-448: Conversion of FoldersThe folders table is updated (lines 443–448) to change _id to TEXT with the proper check constraint.
450-455: Conversion of Folder PathsUpdating the folder_paths table (lines 450–455) to convert _ancestor and _descendant to TEXT—and re‑adding the respective check constraints—is consistent with the overall migration.
457-462: Conversion of Folder Allowed TeamsThe changes (lines 457–462) in folder_allowedTeams are implemented in the same pattern with no issues noted.
464-471: Conversion of Email Verification KeysThis block (lines 464–471) correctly converts _id, email, and _multiUser to TEXT in the emailVerificationKeys table.
472-486: Conversion of AI ModelsThe changes (lines 472–486) update the aiModels table—converting _id, _organization, _dataStore, _user, and trainingJob to TEXT—and re‑adding constraints. The recreation of the aiModels view is done correctly.
487-492: Conversion of AI Model OrganizationsThis brief transaction (lines 487–492) converts _aiModel and _organization to TEXT.
493-498: Conversion of AI Model Training AnnotationsLines 493–498 update both _aiModel and _annotation to TEXT while re‑adding constraints.
500-516: Conversion of AI Inferences and View RecreationThe aiInferences table (lines 500–516) converts several columns (_id, _organization, _aiModel, _newDataset, _annotation, _inferenceJob, newSegmentationLayerName, maskAnnotationLayerName) to TEXT and recreates its view. Given the complexity, please ensure that any code referencing these fields (or the view) is tested thoroughly.
518-526: Conversion of Voxelytics ArtifactsThis transaction (lines 518–526) converts _id, _task, name, path, and version to TEXT in the voxelytics_artifacts table.
528-540: Conversion of Voxelytics RunsThe voxelytics_runs table (lines 528–540) now uses TEXT for identifier fields and other string fields such as workflow_hash and workflow_yamlContent.
542-549: Conversion of Voxelytics TasksConverting _id, _run, name, and task to TEXT in the voxelytics_tasks table (lines 542–549) follows the established pattern.
551-558: Conversion of Voxelytics ChunksThe changes (lines 551–558) update the voxelytics_chunks table by converting _id, _task, executionId, and chunkName to TEXT.
560-564: Conversion of Voxelytics WorkflowsThe transaction (lines 560–564) converts _organization, hash, and name for voxelytics_workflows to TEXT.
566-569: Conversion of Voxelytics RunHeartbeatEventsLines 566–569 update the _run column to TEXT and re‑add its check constraint.
571-574: Conversion of Voxelytics Chunk Profiling EventsThis short transaction (lines 571–574) updates the _chunk column in voxelytics_chunkProfilingEvents to TEXT.
576-583: Conversion of Voxelytics Artifact File Checksum EventsThe changes (lines 576–583) convert _artifact, path, resolvedPath, checksumMethod, and checksum to TEXT.
585-594: Conversion of Analytics EventsThis block (lines 585–594) converts the _id, eventType, _user, _organization, and webknososUri columns in the analyticsEvents table to TEXT.
595-604: Conversion of Credit TransactionsThe transaction covering lines 595–604 updates several identifier columns (_id, _organization, _related_transaction, _paid_job) in the credit_transactions table to TEXT. Pay extra attention to the check constraint on _related_transaction as it is defined against the same 24‑hex pattern.
tools/postgres/schema.sql (4)
1-26: Schema Creation and Release Information VersionIn this schema file the webknossos schema is dropped (if exists) and recreated. Note the change in the releaseInformation table: the INSERT statement (line 24) now sets the schemaVersion to 130. This aligns with the upgrade migration.
28-53: Definition of Custom Types and Core TablesThe file proceeds by defining custom composite types (VECTOR3, BOUNDING_BOX) and ENUMs used later in the schema. Tables such as annotations (starting at line 31) are defined with TEXT-based identifier columns with check constraints ensuring the 24‑hex format. The definitions here already reflect the migration changes being applied elsewhere.
55-251: Comprehensive Table Definitions with TEXT Types and ConstraintsFrom lines 55 onward many table definitions (annotation_layers, annotation_sharedTeams, annotation_contributors, annotation_mutexes, meshes, publications, datasets, dataset_layers, etc.) follow the new design: identifier and related columns are declared as TEXT instead of fixed‑length strings and accompanying default values and check constraints are added.
- The pattern is consistent with the evolutions scripts.
- Views (for example, annotations_, meshes_, publications_) are created to filter out deleted rows.
Please review that the constraint regular expressions (e.g.^[0-9a-f]{24}$) meet your requirements.
253-1041: Remaining Table, View, and Index DefinitionsThe rest of the file (lines 253–1041) contains the definitions for the remaining tables (teams, timespans, organizations, users, tokens, jobs, invites, credentials, folders, etc.) and a very extensive set of foreign key constraints and index definitions. All identifier columns now use TEXT along with the check constraints. The file also defines functions and triggers (starting at line 927) that operate on these tables.
- The triggers and functions (such as onUpdateTask, onInsertAnnotation, onUpdateAnnotation, etc.) appear unaffected by the datatype change, but please verify that any hardcoded assumptions regarding identifier lengths are updated if necessary.
- In addition, the object ID generator function (lines 1017–1038) and the updated hand_out_monthly_free_credits function (lines 1041 and onward) match the new TEXT-based design.
Overall, the schema definition is now in line with the migration objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
conf/evolutions/130-replace-text-types.sql (3)
1-8: Precondition Assertion and CommentsThe script begins with a DO‑block that asserts the releaseInformation’s schema version is 129. This is a proactive check to prevent applying migrations if the existing schema version isn’t as expected. The following block of comments explains the rationale (to break the conversion into multiple transactions) which is clear and informative.
Suggestion: If the schema might be in an indeterminate state, consider using a more descriptive error message or even a conditional “IF … THEN” to offer next steps.
11-30: Annotations Table Transaction BlockThis transaction block drops the view, alters several columns in the
webknossos.annotationstable (changing data types from CHAR/VARCHAR to TEXT) and then re‑establishes check constraints that ensure values match the 24‑character hex (expected ObjectId format). The default values on thenameandtagscolumns are correctly updated as well.Suggestion: Consider using “DROP VIEW IF EXISTS” instead of “DROP VIEW” to make this migration more robust if the view isn’t present.
60-67: Meshes Table BlockThe conversion for the
webknossos.meshestable includes dropping the view, updating the_idand_annotationcolumns to TEXT (with their proper constraints) and then recreating the view filtering onisDeleted.Suggestion: As above, “DROP VIEW IF EXISTS” would improve robustness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
conf/evolutions/130-replace-text-types.sql(1 hunks)tools/postgres/schema.sql(28 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: circleci_build
🔇 Additional comments (67)
conf/evolutions/130-replace-text-types.sql (58)
32-37: Annotation Layers Table BlockThe changes for
webknossos.annotation_layersupdate both the foreign key column_annotation(with its check constraint) and converttracingIdandnameto TEXT. The alterations are consistent with the goal of standardizing identifier types.
39-44: Annotation SharedTeams BlockThe table
annotation_sharedTeamsis updated so that both_annotationand_teamare converted to TEXT and are re‑constrained with the regex check.
46-51: Annotation Contributors BlockSimilar to the previous blocks, this transaction converts
_annotationand_userto TEXT and re‑applies the ObjectId constraints. The changes are straightforward and consistent.
53-58: Annotation Mutexes BlockThis block updates the
annotation_mutexestable by converting the columns_annotationand_userinto TEXT while re‑adding the check constraints.
69-76: Publications Table BlockSimilar to other objects, the script converts
_id,imageUrl, andtitleto TEXT, applies the appropriate check constraints for_id, and then drops/recreates the view for publications.
78-99: Datasets Table BlockAn extensive transaction alters many columns in the
webknossos.datasetstable (e.g._id,_dataStore,_organization,_publicationwith its constraint,_uploader,_folder,name,directoryName,status, etc.). The defaults (for example onstatusandtags) have been updated and a view is recreated afterward. This block is complex but the pattern is applied consistently across every identifier column.
101-106: Dataset Layers BlockThe script converts
_dataset,name, and the array columnmappingsto TEXT (or TEXT[]). The check constraint on_datasetis re‑added.
108-113: Dataset Layer Coordinate Transformations BlockThis transaction updates the
_datasetcolumn (with its check constraint) and converts bothlayerNameandtypeto TEXT.
115-120: Dataset Layer Additional Axes BlockThe table
dataset_layer_additionalAxesis altered by converting_dataset,layerName, andnameto TEXT and adding the check constraint for_dataset.
122-127: Dataset AllowedTeams BlockThe table’s two columns,
_datasetand_team, are updated to TEXT with proper constraints.
129-133: Dataset Mags BlockThe conversion here changes
_datasetanddataLayerNameto TEXT and re‑adds the check constraint for_dataset.
135-140: Dataset Last Used Times BlockBoth
_datasetand_userare converted to TEXT indataset_lastUsedTimes, with appropriate constraints re‑applied.
142-148: Dataset Thumbnails BlockThe block converts
_dataset,dataLayerName,mappingName, andmimetypeto TEXT. This is straightforward and in line with the overall migration.
150-158: DataStores BlockThe script updates the
dataStorestable by dropping the view and converting several key columns (name, url, publicUrl, key, onlyAllowedOrganization) to TEXT.
160-167: TracingStores BlockVery similar to the dataStores block, all relevant columns in
tracingStoresare converted to TEXT and the view is re‑created.
169-180: Projects Table BlockThis block alters columns such as
_id,_organization,_team,_owner, andnamein theprojectstable and updates the view accordingly.
182-191: Scripts Table BlockThe
_id,_owner,name, andgistcolumns in thescriptstable are converted to TEXT with the relevant constraints re‑added.
193-202: TaskTypes Table BlockThe changes update the
_id,_organization,_team, andsummarycolumns in thetaskTypestable; the view is recreated afterward.
204-217: Tasks Table BlockHere the script converts several foreign key columns (
_id,_project,_script,_taskType) and other text fields (e.g.neededExperience_domain,creationInfo) to TEXT. The re‑creation of the view helps maintain application queries.
219-222: Experience Domains BlockBoth the
domainand_organizationcolumns are converted into TEXT.
224-233: Teams and OrganizationTeams BlockThe table
teamsis updated (e.g._id,_organization, andnameare converted to TEXT) and the views for both teams and organizationTeams are recreated.
235-244: Timespans BlockThe timespans table is updated by converting
_id,_user, and_annotationcolumns to TEXT (with check constraints).
246-273: Organizations and UserInfos BlockThis is an extensive block. The script now converts several columns in the
organizationstable (including_id_old,_id,additionalInformation,logoUrl,name,_rootFolder, andnewUserMailingList) to TEXT. It also resets defaults appropriately and then recreates the view for organizations and a composite view (userInfos) that joins users, organizations, and multiUsers.
275-282: Organization Used Storage BlockThe columns in
organization_usedStorageare converted to TEXT (with a check on_dataset).
284-307: Users BlockThe script alters the
userstable by converting_id,_multiUser,_organization,firstName,lastName, andlastTaskTypeIdto TEXT and adds a check constraint forlastTaskTypeId. The users view and the associateduserInfosview are then recreated.
309-314: User Team Roles BlockBoth
_userand_teamin this table are now TEXT with proper constraints.
316-320: User Experiences BlockThe changes here convert the
_useranddomaincolumns to TEXT and add the constraint.
322-327: User Dataset Configurations BlockThe identifiers
_userand_datasetare altered to TEXT here and constraints are re‑applied.
329-335: User Dataset Layer Configurations BlockThe columns
_user,_dataset, andlayerNameare updated to TEXT.
337-357: MultiUsers and Re‑created UserInfos BlockThe multiUsers table is updated with conversion of
_id,passwordInfo_password, and_lastLoggedInIdentityto TEXT (with check constraints). The views are then recreated using the updated multiUsers table.
359-365: Tokens BlockThe
_idandloginInfo_providerKeycolumns are changed to TEXT in thetokenstable. The view is re‑created.
367-374: Maintenances BlockThe table
maintenanceshas its_idand_usercolumns converted to TEXT.
376-387: Workers BlockThe workers table conversion changes several columns (_id, _dataStore, name, key, supportedJobCommands) to TEXT. A default value for the worker’s name remains and the view is recreated.
389-403: Jobs BlockAll identifier columns in
jobs(_id, _owner, _dataStore, _worker, _voxelytics_workflowHash, latestRunId, returnValue) are updated to TEXT. The respective check constraint(s) are re‑added and the view is recreated.
405-411: Invites BlockThe
_idand_organizationcolumns in theinvitestable are now TEXT and the view is re‑created.
413-421: Annotation Private Links BlockConverts
_idand_annotationto TEXT and updates the view accordingly.
423-428: ShortLinks BlockBoth
_idand the key columns inshortLinksare converted to TEXT.
430-441: Credentials BlockThe credentials table now stores
_id,name,identifier,secret,_user, and_organizationas TEXT. Note that this affects how credential data is handled—but the conversion appears straightforward.
443-448: Folders BlockThe
_idcolumn in thefolderstable is converted to TEXT and the view is recreated.
450-455: Folder Paths BlockBoth
_ancestorand_descendantare converted to TEXT with newly added check constraints.
457-462: Folder Allowed Teams BlockThe foreign key columns are updated to TEXT here too.
464-471: Email Verification Keys BlockConverts
_id,_multiUserto TEXT and re‑adds the check constraint on_multiUser.
472-485: AI Models BlockFor the
aiModelstable, all identifier columns (_id, _organization, _dataStore, _user, _trainingJob) are now TEXT with their check constraints re‑applied. The comment column is also converted to TEXT. The corresponding view is created.
487-491: AI Model Organizations BlockConverts
_aiModeland_organizationto TEXT.
493-498: AI Model TrainingAnnotations BlockUpdates the
_aiModeland_annotationcolumns, ensuring they are TEXT with check constraints.
500-516: AI Inferences BlockThis large block converts many columns in
aiInferences(including all foreign keys and new segmentation/mask layer names) to TEXT and re‑adds the check constraints. The associated view is recreated.
518-526: Voxelytics Artifacts BlockConverts the identifier and name columns (and the version) to TEXT in the
voxelytics_artifactstable.
528-540: Voxelytics Runs BlockAll identifier columns and text fields (such as
workflow_hashandworkflow_yamlContent) invoxelytics_runsare updated to TEXT.
542-549: Voxelytics Tasks BlockUpdates the
_idand_runidentifier columns as well as the other key attributes to TEXT.
551-558: Voxelytics Chunks BlockThis transaction converts several columns (_id, _task, executionId, chunkName) to TEXT and adds the required constraints.
560-564: Voxelytics Workflows BlockBoth the
_organizationand the hash and name columns are updated to TEXT.
566-569: Voxelytics RunHeartbeatEvents BlockThe
_runcolumn is converted to TEXT and the check constraint is re‑added.
571-574: Voxelytics Chunk Profiling Events BlockUpdates the
_chunkcolumn to TEXT with the proper check constraint.
576-583: Voxelytics Artifact File Checksum Events BlockThis block updates the
_artifact,path,resolvedPath,checksumMethod, andchecksumcolumns to TEXT.
585-593: Analytics Events BlockThe
analyticsEventstable is updated so that_id,eventType,_user,_organization, andwebknossosUribecome TEXT.
595-607: Credit Transactions BlockThe block converts several identifier columns (_id, _organization, _related_transaction, _paid_job) to TEXT (with constraints) and resets defaults where needed; the view is recreated based on a different “is_deleted” flag.
609-618: Updated Function: hand_out_monthly_free_creditsThe function has been modified so that the internal variable (and the parameter choice) uses TEXT for
organization_idinstead of VARCHAR. The loop selecting organizations and the insertion intocredit_transactionsnow use the new TEXT type. The logic all remains clear and consistent.
639-640: Schema Version UpdateThe final UPDATE statement sets the schema version to 130 in the releaseInformation table. This marks the successful application of this migration.
tools/postgres/schema.sql (9)
18-26: Release Information and Schema VersionThe schema creation begins with the DROP and CREATE of the
webknossosschema, then creates thereleaseInformationtable and immediately inserts the new schema version (130). This correctly reflects the migration update.
32-44: Annotations Table DefinitionWithin the table definition for
annotations, notice that all identifier columns (such as_id,_dataset,_task,_team,_user, and_publication) are now defined as TEXT with a check constraint enforcing a 24‑hex digit format. Additionally, the default values for thenameandtagscolumns have been set appropriately.
55-65: Annotation Layers DefinitionThe
annotation_layerstable creation reflects similar changes with columns_annotationandnameupdated to TEXT and appropriate constraints in place.
67-77: Annotation SharedTeams and ContributorsThe subsequent definitions (for sharedTeams and contributors) similarly adopt TEXT for foreign key columns along with check constraints. This uniformity is crucial and looks correct.
79-103: Meshes and Publications DefinitionsThe tables such as
meshesandpublicationsnow use TEXT for identifiers and other string fields (including URL, title, etc.). The changes carry over the check constraints and default values as expected.
105-135: Datasets and Related TablesThe
datasetstable (and its related tables such as dataset_layers, coordinateTransformations, additionalAxes, allowedTeams, mags, lastUsedTimes, and thumbnails) are all now defined with TEXT columns and matching constraints. This comprehensive update in the schema file mirrors the evolution script, ensuring full consistency across the database.
137-275: Remaining Table Definitions (DataStores, TracingStores, Projects, Scripts, TaskTypes, Tasks, Experience Domains, Teams, Timespans, Organizations, Users, etc.)Most of the remaining part of the schema file shows the updated definitions. All identifier columns have been switched to TEXT and proper check constraints are applied. In several cases (such as users, multiUsers, tokens, etc.), related views are created too. The uniformity of these changes is exactly in line with the PR objectives.
275-607: Comprehensive Updates Through the SchemaFrom
organizationstocredit_transactionsand beyond, every table and view reflects the conversion that was implemented in the evolution script. This schema file appears auto‑generated (or updated in tandem) to match the new state of the database.
609-640: Supporting Functions and Final AdjustmentsThe file also defines functions (such as the ObjectId generator and the updated
hand_out_monthly_free_credits) that use TEXT for identifiers. All triggers, indexes, and foreign key constraints have been updated accordingly. Overall, the schema is consistent and aligned with the migration script.
Fixes regression from #8372 ### URL of deployed dev instance (used for testing): - https://movefolders.webknossos.xyz ### Steps to test: - Create a couple of folders in dashboard - Move them into each other, should work and not error ### Issues: - fixes #8777 ------ - [x] Added changelog entry (create a `$PR_NUMBER.md` file in `unreleased_changes` or use `./tools/create-changelog-entry.py`) - [x] Considered [common edge cases](../blob/master/.github/common_edge_cases.md)
3 participants
URL of deployed dev instance (used for testing):
This is the first batch of changes for #8191
I could readd the constraints of length to all the new text types that were char(n) before if that seems useful.
Issues:
(Please delete unneeded items, merge only when none are left open)
Needs datastore update after deployment