-
Notifications
You must be signed in to change notification settings - Fork 11
fix: serialize array field #675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #675 +/- ##
==========================================
- Coverage 92.60% 92.54% -0.07%
==========================================
Files 1296 1296
Lines 47640 47671 +31
Branches 1592 1601 +9
==========================================
Hits 44118 44118
- Misses 3213 3244 +31
Partials 309 309
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| elements.append(str(item)) | ||
| else: | ||
| cleaned = str(item).replace("\x00", "") | ||
| elements.append(cleaned) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nested ArrayField produces invalid PostgreSQL array syntax
Medium Severity
The _serialize_pg_array function doesn't handle nested arrays (Django's ArrayField(ArrayField(...)) pattern). When an array element is itself a list, it falls through to the else branch and produces Python list syntax [1, 2, 3] instead of PostgreSQL array syntax {1, 2, 3}. This generates completely invalid SQL that would fail on import. Nested/multidimensional arrays require recursive serialization to produce the correct PostgreSQL format like '{{1,2},{3,4}}'::integer[][].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is okay, we don't have nested ArrayField(ArrayField(...)) in the codebase
Adjust how we're serializing arrayfields with the sql_generator
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Note
Improves SQL export serialization to correctly handle Django
ArrayFields.serialize_valuenow acceptsfieldand serializes lists as PostgreSQL arrays when the field is anArrayField; otherwise lists/dicts are serialized asjsonb_serialize_pg_arraybuilds typed PG array literals with proper escaping and casts (e.g.,::integer[],::text[])fieldcontext;get_row_valuespasses model fields toserialize_valueWritten by Cursor Bugbot for commit 903200d. This will update automatically on new commits. Configure here.