fix(db): change state_data column from jsonb to text to fix 22P02 error - #9
Conversation
There was a problem hiding this comment.
Code Review
This pull request changes the state_data column type from JSONB to TEXT in the database initialization script and the application's database context. Feedback highlights that the SQL change won't apply to existing databases due to the IF NOT EXISTS clause and may cause base64 decoding errors because existing JSON quotes are preserved during the type cast. Additionally, storing large binary data as Base64 in a text column is noted as inefficient, with suggestions to use BYTEA or external file storage instead.
| document_id UUID NOT NULL, | ||
| current_activity VARCHAR(128) NOT NULL, | ||
| state_data JSONB, | ||
| state_data TEXT, |
There was a problem hiding this comment.
Changing the column type in init.sql will not update existing databases where the table already exists due to the IF NOT EXISTS clause. Additionally, a simple type cast from JSONB to TEXT in an ALTER TABLE statement will preserve JSON quotes (e.g., storing "SGVsbG8=" instead of SGVsbG8=). This will cause Convert.FromBase64String to fail in DocumentProcessingAgent.cs (line 82) when the agent attempts to resume from a checkpoint created before this change.
Recommendation: Ensure a migration script is provided that both alters the type and unquotes existing data:
ALTER TABLE workflow_checkpoints ALTER COLUMN state_data TYPE TEXT USING state_data#>>'{}';| entity.Property(e => e.AgentName).HasMaxLength(128).IsRequired(); | ||
| entity.Property(e => e.CurrentActivity).HasMaxLength(128).IsRequired(); | ||
| entity.Property(e => e.StateData).HasColumnType("jsonb"); | ||
| entity.Property(e => e.StateData).HasColumnType("text"); |
There was a problem hiding this comment.
While changing the column type to text resolves the 22P02 (invalid JSON) error, storing large binary data as Base64 in a database column is inefficient. Base64 encoding increases the payload size by approximately 33%, leading to significant database bloat and increased memory pressure when checkpoints are retrieved.
Since the DocumentId and FilePath are already available in the AgentContext, consider refactoring the agent to re-retrieve the file from IFileStorage upon resume instead of serializing the entire PDF into the database. If storing the payload is strictly necessary, a BYTEA column or a dedicated blob storage would be more performant.
No description provided.