kolofordjango / kolo

See everything happening in your running Django app. All without leaving VSCode
https://kolo.app
504 stars 14 forks source link

Error when trying to open a db query #64

Closed DentaCool closed 1 year ago

DentaCool commented 1 year ago

VSCODE: "Invalid query argument. Extected string, instead got object"

I also added the log of the object itself to the throw via vim:

"Invalid query argument. Extected string, instead got object: null"

UPD: FRAME: file 'python3.9/site-packages/django/db/models/sql/compiler.py', line 1161, code execute_sql>

def execute_sql(self, result_type=MULTI, chunked_fetch=False, chunk_size=GET_ITERATOR_CHUNK_SIZE):
        result_type = result_type or NO_RESULTS
        try:
            sql, params = self.as_sql()
            if not sql:
                raise EmptyResultSet
        except EmptyResultSet:
            if result_type == MULTI:
                return iter([])
            else:
                return

UPD 2:

from psqlextra.query import PostgresQuerySet

wilhelmklopp commented 1 year ago

@LilyFoote any ideas here?

Possibly we don't support PostgresQuerySet?

@DentaCool do you use PostgresQuerySet directly? Does the issue happen even when you don't use it?

LilyFoote commented 1 year ago

Hard to say for sure without looking closely myself, but I wonder if we're just recording the query as null because we can't introspect it as something more useful.

@DentaCool Can you share the code that's triggering this when run with Kolo? Ideally a minimised example, so there's not too much code to work with, but I'll take what you can get.

DentaCool commented 1 year ago
class SQLQueryFilter:
  def process(
          self,
          frame: types.FrameType,
          event: str,
          arg: Any,
          call_frames: List[Tuple[types.FrameType, str]],
      ):
      # ...
        try:
            sql = frame.f_locals["sql"]
            params = frame.f_locals["params"]
        except KeyError:
            query_template = None
            query = None
        # ...

django/db/models/sql/compiler.py

def execute_sql():
        try:
            sql, params = self.as_sql()
            if not sql:
                raise EmptyResultSet
        except EmptyResultSet:
            if result_type == MULTI:
                return iter([])

this function fails with EmptyResultSet and returns iter([]). Thus

sql = frame.f_locals["sql"]
params = frame.f_locals["params"]

also fails with a KeyError and we will get

{
    "database": "postgres",
    "query": null,
    "query_template": null,
    "query_data": "<list_iterator object at 0x00B99F28>",
    "type": "end_sql_query"
}

At the moment, I'm just hardcoded fake query_template, query and query_data at KeyError. I will try to find more information and a solution as soon as I have time

LilyFoote commented 1 year ago

That's very interesting! Do you know which queryset in your code ends up with the EmptyResultSet exception?

DentaCool commented 1 year ago
from psqlextra.manager import PostgresManager, PostgresQuerySet

class SomeModelQuerySet(PostgresQuerySet):
    """Some query logic"""

class SomeModelManager(PostgresManager):
    def get_queryset(self):
        return SomeModelQuerySet(self.model, using=self._db)

class SomeModel(Model):
    # some fields and relations
    objects = SomeModelManager()

qs = SomeModel.objects.filter(some_relation=some_relation, other_field__in=[1,2,3]).prefetch_related('other_relation')

some_mapper = {instance.pk: some_func(instance)  for instance in qs}

Here is the code that was executed (as far as I remember). I will try to provide more detailed information after 13:00 Kyiv time.

Perhaps it would be nice to create a repository for the python library in future so that it is possible to contribute code

LilyFoote commented 1 year ago

Thanks! I'll look into that properly tomorrow.

LilyFoote commented 1 year ago

Looks like this is caused by Model.objects.none() or similar. In that case, there isn't a query to record, so Kolo stores null instead. I think this is something we need to handle better in VSCode.

LilyFoote commented 1 year ago

@wilhelmklopp ~Actually, we could just ignore this case on the python side.~ I'm not sure if it's valuable to display empty queries like this.

Edit: Actually, I think we need to emit the end frame in this case, since we can't detect we're in the empty queryset case when processing the start frame, so we can't drop both.

wilhelmklopp commented 1 year ago

👋 @DentaCool the latest version of the VSCode extension (2.8.1) includes the fix for this :)