adamchainz / django-perf-rec

Keep detailed records of the performance of your Django code.
MIT License
347 stars 25 forks source link

Interaction between Django-perf-rec and Sql commenter #590

Open cosmith opened 10 months ago

cosmith commented 10 months ago

Python Version

3.11.0

Django Version

4.2.2

Package Version

4.24

Description

Hello, thanks for this project!

I just ran into a little issue when moving our tests from TransactionTestCase to TestCase: we now have savepoints in our recordings.

Since we use SQL Commenter to have more info in our query plans on Google Cloud SQL, our SQL queries look like this:

ROLLBACK TO SAVEPOINT "s139987847644992_x3209" /*controller='v4%%3Atransports-update-loads-constraints',route='api/v4/transports/%%28%%3FP%%3Cuid%%3E%%5B%%5E/%%5D%%2B%%29/loads-constraints/%%24'*/

The code at https://github.com/adamchainz/django-perf-rec/blob/main/src/django_perf_rec/sql.py#L123 checks that len(node.tokens) == 7 which is not the case for us because of the comments.

I have tested removing the length condition and it works fine.

I'm happy to submit a PR, however there might be other ways of fixing this:

Let me know what you think!

adamchainz commented 9 months ago

I think go ahead and open a PR. Please provide a test and changelog note. I’d rather not strip comments since they probably contain useful info!