Skip to content
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

Query builder queries are not immutable #550

Open
2 tasks done
jscheel opened this issue Dec 19, 2024 · 14 comments
Open
2 tasks done

Query builder queries are not immutable #550

jscheel opened this issue Dec 19, 2024 · 14 comments
Assignees
Labels
bug Something isn't working

Comments

@jscheel
Copy link

jscheel commented Dec 19, 2024

Bug report

  • I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

Query builder objects are not immutable, which means that you cannot share a query builder and add separate conditions on each execution.

query = (
    sb.table("items")
    .select("*")
    .eq("account_id", "abc")
)
query.in_("id", ["1", "2", "3"]).execute()
query.in_("id", ["4", "5", "6"]).execute()

This will execute one query with ids = 1, 2, 3 and then the second query will be 1, 2, 3, 4, 5, 6.

Expected behavior

One query with ids = 1, 2, 3 and another with 4, 5, 6.

Discord Discussion

https://discord.com/channels/839993398554656828/1319331194902413384

@jscheel jscheel added the bug Something isn't working label Dec 19, 2024
@silentworks silentworks self-assigned this Dec 19, 2024
@silentworks
Copy link
Contributor

I've tested this and I'm unable to reproduce it. I get an issue of when one query runs the other doesn't run at all and not the query appending to each other.

@jscheel
Copy link
Author

jscheel commented Dec 25, 2024

@silentworks I'll shoot you a full, reproducible case when I get a moment. Maybe a version mismatch or something.

@silentworks
Copy link
Contributor

I created a reproducible with the results I'm getting. You can modify it to see if you can get the results you got and create a PR to the repo https://github.com/silentworks/postgres_py_issue_550

@silentworks
Copy link
Contributor

@jscheel did you get around looking at the version I created? or creating a reproducible example repo of the behavior you are getting?

@silentworks
Copy link
Contributor

Closing this out as no reply from OP over a month now. Also unable to reproduce the issue.

@silentworks silentworks added invalid This doesn't seem right and removed bug Something isn't working labels Jan 29, 2025
@jscheel
Copy link
Author

jscheel commented Jan 29, 2025 via email

@DDoerner
Copy link

DDoerner commented Apr 1, 2025

The issue is very noticable when trying to do pagination using a single base query.

We initially built pagination like this:

query = client.table(table_name).select("*")
items = []
for offset in range(0, MAX_ROWS, PAGE_SIZE):
  next_page = query.range(offset, offset + PAGE_SIZE).execute()
  if next_page.data:
    items.extend(next_page.data)
  ...

But we noticed this actually attempts to repeatedly add the offset and limit parameters to the query object, producing HTTP calls such as ?select=*&offset=0&offset=250&offset=500&offset=750&offset=1000&offset=1250&offset=1500&offset=1750&offset=2000&offset=2250&limit=251&limit=251&limit=251&limit=251&limit=251&limit=251&limit=251&limit=251&limit=251&limit=251.

@silentworks
Copy link
Contributor

@DDoerner I've left a repo above showing that I cannot reproduce what the OP posted. Can you clone it and update it to show your issue and then I'll have a look.

@jscheel
Copy link
Author

jscheel commented Apr 3, 2025

@silentworks The bug still appears to exist. I've created a reproducible repo for you here: https://github.com/jscheel/supabase-query-builder-bug

I enabled logging so that you can see the two requests, with the second one getting both conditions. Sorry I didn't fork your repo, had this example before I saw your repo 😅

@silentworks
Copy link
Contributor

The example your provided doesn't have any data to go with it. This is why I created the example repo as it has data and everything. I'm going to update your example to use data similar to what you originally posted.

Ok tested this and can now replicate the issue. I will reopen this and start looking into fixing this now.

@silentworks silentworks reopened this Apr 6, 2025
@jscheel
Copy link
Author

jscheel commented Apr 6, 2025

@silentworks yeah, sorry, like I said, I didn't realize. Was just trying to bang something out quickly (deep in some other work atm), but any table with a id::uuid column should work. Lemme know if I can give you any more details!

@silentworks
Copy link
Contributor

I see what the issue is but fixing this is going to create a breaking change which I don't think we will want to do. I tested out some other options which would work for the chaining from the .table as .select being chained means that all filters are appended.

query = sb.table("items")
query.select("*").eq("account_id", "abc").in_("id", ["1", "2", "3"]).execute()
query.select("*").eq("account_id", "abc").in_("id", ["4", "5", "6"]).execute()

or if you use the copy module

from copy import copy

query = (
    sb.table("items")
    .select("*")
    .eq("account_id", "abc")
)
copy(query).in_("id", ["1", "2", "3"]).execute()
copy(query).in_("id", ["4", "5", "6"]).execute()

@jscheel
Copy link
Author

jscheel commented Apr 6, 2025

If you are concerned about backwards compatibility, then probably best just to show a warning if execute is called a second time but the resolved query does not match the first request. If select is what actually resolves the query, it's possible that could be tacked on the end if someone needs to create a new query from the chain (not sure if that's actually true, not at my computer to test). Otherwise, this probably just a warning and documentation solution. I believe the documented options would be:

  1. Write new query
  2. Use a query factory (just a cleaner number 1)
  3. Use copy

correct?

@silentworks
Copy link
Contributor

Yes those are the options. I am going to leave this issue open as you have made good suggestions here. I will close this when we have decided on which step to take and update you here.

@silentworks silentworks added bug Something isn't working and removed invalid This doesn't seem right labels Apr 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants