Skip to content

Move to JSONB#2270

Merged
glennjacobs merged 14 commits into1.xfrom
move-to-jsonb
Aug 15, 2025
Merged

Move to JSONB#2270
glennjacobs merged 14 commits into1.xfrom
move-to-jsonb

Conversation

@glennjacobs
Copy link
Contributor

@glennjacobs glennjacobs commented Aug 14, 2025 •
edited by coderabbitai bot
Loading

Closes #2261

We were using standard JSON fields and when it comes to Postgres this field type is very basic, not much more than a simple text field.

JSONB is a "proper" JSON field, is supported properly in Filament and should perform a lot better.

It's worth noting MySQL & Sqlite will continue to use the same JSON field type - so no change there.

Summary by CodeRabbit

  • Refactor

    • Converted many metadata and attribute fields to PostgreSQL JSONB for improved performance and consistency across activity logs, addresses, attributes, brands, carts, collections, customers, discounts, orders, products, shipping, transactions, and media.

  • Chores

    • Added a PostgreSQL-only migration to perform the conversions and raised the PostgreSQL minimum requirement from 9.2+ to 9.4+.

@vercel
Copy link

vercel bot commented Aug 14, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
lunar-docs ✅ Ready Preview Comment Aug 15, 2025 9:06am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 14, 2025 •
edited
Loading

Walkthrough

Adds a PostgreSQL-only Laravel migration converting many JSON columns to JSONB (with per-column nullability and reversible down()), and updates installation docs to require PostgreSQL 9.4+.

Changes

Cohort / File(s) Summary
DB migration: JSON → JSONB switch
packages/core/database/migrations/2025_08_14_164000_switch_to_jsonb.php		 New migration with private $columnsToUpdate mapping (tables → columns + nullable flags); up() converts listed columns to jsonb(...) for pgsql only and applies nullability; down() reverts to json(...); includes getTableName() to resolve prefixed/special table names.
Docs: PostgreSQL version requirement
docs/core/installation.md		 Updated PostgreSQL minimum version requirement from 9.2+ to 9.4+ in installation requirements.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Assessment against linked issues

Objective Addressed Explanation
Convert collection attribute_data (and related JSON columns) to JSONB to resolve Postgres "could not identify an equality operator for type json" error [#2261] ✅

Out-of-scope changes

Code Change Explanation
PostgreSQL minimum version bump (docs/core/installation.md) Documentation-only change; not required to implement the migration objective in the linked issue.
📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cf4661e and 78f0ef0.

📒 Files selected for processing (1)
  • packages/core/database/migrations/2025_08_14_164000_switch_to_jsonb.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/database/migrations/2025_08_14_164000_switch_to_jsonb.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: core - PHP 8.4 - L11.* ↑
  • GitHub Check: shipping - PHP 8.4 - L12.* ↑
  • GitHub Check: shipping - PHP 8.4 - L12.* ↑ E
  • GitHub Check: search - PHP 8.4 - L12.* ↑
  • GitHub Check: core - PHP 8.4 - L12.* ↑
  • GitHub Check: stripe - PHP 8.4 - L11.* ↑
  • GitHub Check: shipping - PHP 8.3 - L12.* ↑
  • GitHub Check: admin - PHP 8.4 - L12.* ↑
  • GitHub Check: search - PHP 8.4 - L11.* ↑
  • GitHub Check: stripe - PHP 8.3 - L12.* ↑ E
  • GitHub Check: admin - PHP 8.4 - L11.* ↑
  • GitHub Check: core - PHP 8.3 - L12.* ↑
  • GitHub Check: admin - PHP 8.3 - L12.* ↑ E
  • GitHub Check: admin - PHP 8.3 - L12.* ↑
  • GitHub Check: core - PHP 8.3 - L12.* ↑ E
  • GitHub Check: admin - PHP 8.3 - L11.* ↑
  • GitHub Check: core - PHP 8.3 - L11.* ↑
  • GitHub Check: admin - PHP 8.3 - L11.* ↑ E
  • GitHub Check: core - PHP 8.3 - L11.* ↑ E
  • GitHub Check: fix-code-style
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch move-to-jsonb

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@glennjacobs glennjacobs marked this pull request as ready for review August 14, 2025 17:18
@glennjacobs glennjacobs requested review from alecritson and removed request for alecritson August 14, 2025 17:18
@glennjacobs glennjacobs marked this pull request as draft August 14, 2025 17:19
coderabbitai[bot]

This comment was marked as outdated.

Sign in to view

@glennjacobs glennjacobs marked this pull request as ready for review August 14, 2025 17:39
@glennjacobs glennjacobs requested a review from alecritson August 14, 2025 17:39
coderabbitai[bot]
coderabbitai bot reviewed Aug 14, 2025
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
docs/core/installation.md (1)

12-12: Clarify that JSONB is required to avoid ambiguity.

Explicitly calling out JSONB will help users understand why 9.4+ is needed.

- - MySQL 8.0+ / PostgreSQL 9.4+
+ - MySQL 8.0+ / PostgreSQL 9.4+ (JSONB required)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4495aa4 and 1e2f263.

📒 Files selected for processing (1)
  • docs/core/installation.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/core/installation.md

[grammar] ~12-~12: There might be a mistake here.
Context: ...el 11, 12 - MySQL 8.0+ / PostgreSQL 9.4+ - exif PHP extension (on most systems it w...

(QB_NEW_EN)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: core - PHP 8.4 - L12.* ↑
  • GitHub Check: stripe - PHP 8.4 - L11.* ↑ E
  • GitHub Check: admin - PHP 8.4 - L11.* ↑ E
  • GitHub Check: core - PHP 8.4 - L11.* ↑ E
  • GitHub Check: admin - PHP 8.4 - L11.* ↑
  • GitHub Check: admin - PHP 8.3 - L12.* ↑
  • GitHub Check: core - PHP 8.3 - L12.* ↑ E
  • GitHub Check: core - PHP 8.3 - L11.* ↑ E
  • GitHub Check: admin - PHP 8.3 - L11.* ↑ E
  • GitHub Check: admin - PHP 8.3 - L11.* ↑
  • GitHub Check: core - PHP 8.3 - L11.* ↑
  • GitHub Check: fix-code-style
🔇 Additional comments (1)
docs/core/installation.md (1)

12-12: PostgreSQL 9.4+ aligns with the JSONB migration—good update.

JSONB was introduced in PostgreSQL 9.4, so bumping the minimum version to 9.4+ is necessary for the new migration that converts JSON to JSONB.

alecritson
alecritson reviewed Aug 15, 2025
View reviewed changes
coderabbitai[bot]
coderabbitai bot reviewed Aug 15, 2025
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
packages/core/database/migrations/2025_08_14_164000_switch_to_jsonb.php (1)

108-108: Replace jsonb() with json() or add driver-specific guards.

The migration uses Laravel's jsonb() method which is PostgreSQL-specific and will fail on MySQL. While the current early return prevents execution on non-PostgreSQL drivers, this creates an inconsistency where the up() and down() methods use different column types (jsonb vs json).

For consistency and to avoid potential future issues, consider using json() in both methods or adding explicit driver checks around the jsonb() calls:

-$columnBuilder = $tableBlueprint->jsonb($column['name']);
+$columnBuilder = $tableBlueprint->json($column['name']);

Also applies to: 137-137

🧹 Nitpick comments (1)
packages/core/database/migrations/2025_08_14_164000_switch_to_jsonb.php (1)

163-164: Remove unnecessary blank line.

There's an extra blank line before the return statement.

-        
 return $this->prefix.$table;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1e2f263 and 6b7be27.

📒 Files selected for processing (1)
  • packages/core/database/migrations/2025_08_14_164000_switch_to_jsonb.php (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/database/migrations/2025_08_14_164000_switch_to_jsonb.php (1)
packages/core/src/Base/Migration.php (1)
  • Migration (7-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: search - PHP 8.4 - L11.* ↑ E
  • GitHub Check: stripe - PHP 8.4 - L12.* ↑ E
  • GitHub Check: stripe - PHP 8.4 - L11.* ↑
  • GitHub Check: admin - PHP 8.4 - L12.* ↑
  • GitHub Check: core - PHP 8.4 - L12.* ↑
  • GitHub Check: admin - PHP 8.4 - L11.* ↑
  • GitHub Check: stripe - PHP 8.3 - L12.* ↑
  • GitHub Check: core - PHP 8.4 - L11.* ↑
  • GitHub Check: core - PHP 8.4 - L12.* ↑ E
  • GitHub Check: stripe - PHP 8.3 - L12.* ↑ E
  • GitHub Check: core - PHP 8.3 - L12.* ↑
  • GitHub Check: admin - PHP 8.3 - L12.* ↑
  • GitHub Check: admin - PHP 8.3 - L12.* ↑ E
  • GitHub Check: admin - PHP 8.3 - L11.* ↑
  • GitHub Check: admin - PHP 8.3 - L11.* ↑ E
  • GitHub Check: core - PHP 8.3 - L12.* ↑ E
  • GitHub Check: core - PHP 8.3 - L11.* ↑
  • GitHub Check: core - PHP 8.3 - L11.* ↑ E
  • GitHub Check: fix-code-style
🔇 Additional comments (4)
packages/core/database/migrations/2025_08_14_164000_switch_to_jsonb.php (4)

98-101: Guard logic correctly restricts JSONB migration to PostgreSQL only.

The driver check ensures the migration only runs on PostgreSQL, maintaining compatibility with MySQL and SQLite as stated in the PR objectives.

13-91: Comprehensive column mapping covers all relevant JSON fields.

The mapping correctly identifies all JSON columns that need conversion to JSONB, including the critical attribute_data fields in collections table that were causing the PostgreSQL equality operator issue mentioned in #2261. The nullable flags are properly configured for each column.

154-165: Table name resolution correctly handles special cases.

The getTableName method properly handles tables that don't use the lunar_ prefix (activity_log, media) and correctly applies the configured prefix to Lunar-specific tables.

125-149: Down migration correctly reverses the JSONB conversion.

The rollback logic properly converts JSONB columns back to JSON while preserving the original nullability constraints. The PostgreSQL driver check ensures consistency with the up() method.

@glennjacobs glennjacobs requested a review from alecritson August 15, 2025 08:44
@glennjacobs glennjacobs requested a review from alecritson August 15, 2025 09:06
@glennjacobs glennjacobs merged commit be7f0c5 into 1.x Aug 15, 2025
48 checks passed
@glennjacobs glennjacobs deleted the move-to-jsonb branch August 15, 2025 09:12
This was referenced Aug 17, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alecritson alecritson alecritson approved these changes

+2 more reviewers

@wychoong wychoong wychoong left review comments

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cannot attach collection from product detail (Postgres Issue)

4 participants

@glennjacobs @alecritson @wychoong @actions-user