fix: PostgreSQL compatibility — explicit boolean comparisons - #2547
Open
skanderphilipp wants to merge 2 commits into
Open
fix: PostgreSQL compatibility — explicit boolean comparisons#2547skanderphilipp wants to merge 2 commits into
skanderphilipp wants to merge 2 commits into
Conversation
Fix two PostgreSQL errors: 1. crm_service_level_agreement/utils.py: SLA.enabled is a smallint (Check field). PostgreSQL requires explicit comparison in WHERE clauses; MariaDB silently coerces smallint to boolean. Changed .where(SLA.enabled) to .where(SLA.enabled == 1). 2. crm_notification/crm_notification.py: the values dict passed to frappe.db.exists() contained a 'doctype' key that does not correspond to a column in tabCRM Notification. PostgreSQL rejects unknown columns in WHERE clauses, while MariaDB's behavior depends on sql_mode. Stripped the key before the existence check. Both fixes are backwards-compatible with MariaDB.
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
Contributor
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (2): Last reviewed commit: "fix: PostgreSQL — demo data case sensiti..." | Re-trigger Greptile |
Two fixes for v17/develop with PostgreSQL backend: 1. Demo data used 'Budget constraints' but the actual Lost Reason record is named 'Budget Constraints' (uppercase C). PostgreSQL string comparison is case-sensitive by default, while MariaDB uses case-insensitive collation. Fixed to match exact record name. 2. CRM Lost Reasons are required by demo data but were only defined in test_records.json (loaded during tests). Added them as app fixtures so they are created during bench install-app crm, ensuring demo data works on fresh installations. Both fixes are backwards-compatible with MariaDB.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2547 +/- ##
========================================
Coverage 67.46% 67.47%
========================================
Files 173 173
Lines 11620 11622 +2
========================================
+ Hits 7840 7842 +2
Misses 3780 3780
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix two PostgreSQL errors found during Frappe v17 (develop) setup:
1.
crm_service_level_agreement/utils.py— bare smallint in WHERESLA.enabledis aCheckfield stored assmallint. The query builder generatedWHERE enabledwithout a comparison operator. MariaDB silently coerces smallint → boolean, but PostgreSQL requires explicit comparison.Fix:
.where(SLA.enabled)→.where(SLA.enabled == 1)2.
crm_notification/crm_notification.py— unknown column in filterThe
valuesdict contained a"doctype"key intended forfrappe.get_doc()routing, but the same dict was passed tofrappe.db.exists()which interprets all keys as WHERE filter columns. SincetabCRM Notificationhas nodoctypecolumn, PostgreSQL errors.Fix: Strip the
doctypekey before passing tofrappe.db.exists().Both fixes are backwards-compatible with MariaDB and follow the existing Frappe query builder patterns.