Skip to content

fix: handle queries on non-existing table gracefully - #3869

Merged
steve-chavez merged 2 commits into
PostgREST:mainfrom
taimoorzaeem:non-existing-table/issue3697
Feb 21, 2025
Merged

fix: handle queries on non-existing table gracefully#3869
steve-chavez merged 2 commits into
PostgREST:mainfrom
taimoorzaeem:non-existing-table/issue3697

Conversation

@taimoorzaeem

@taimoorzaeem taimoorzaeem commented Jan 21, 2025

Copy link
Copy Markdown
Member

Add new TableNotFound error and add json error
message for this error. Closes #3697, closes #3602.

@taimoorzaeem
taimoorzaeem force-pushed the non-existing-table/issue3697 branch from 95d4973 to 52591c0 Compare January 21, 2025 14:26
Comment thread CHANGELOG.md Outdated
Comment thread test/io/test_io.py
Comment thread test/spec/Feature/Query/InsertSpec.hs Outdated
Comment thread test/spec/Feature/Query/QuerySpec.hs Outdated
@steve-chavez

Copy link
Copy Markdown
Member

Now that we rely on the schema cache for simple table filters, we need to make sure these requests are fast when a schema cache load for relationships is slow (#3046).

There's a test that validates the above behavior (a request with resource embedding is blocked until the schema cache finishes loading):

def test_requests_wait_for_schema_cache_reload(defaultenv):
"requests that use the schema cache (e.g. resource embedding) wait for the schema cache to reload"
env = {
**defaultenv,
"PGRST_DB_SCHEMAS": "apflora",
"PGRST_DB_POOL": "2",
"PGRST_DB_ANON_ROLE": "postgrest_test_anonymous",
"PGRST_SERVER_TIMING_ENABLED": "true",
}
with run(env=env, wait_max_seconds=30) as postgrest:
# reload the schema cache
response = postgrest.session.get("/rpc/notify_pgrst")
assert response.status_code == 204
postgrest.wait_until_scache_starts_loading()
response = postgrest.session.get("/tpopmassn?select=*,tpop(*)")
assert response.status_code == 200
plan_dur = parse_server_timings_header(response.headers["Server-Timing"])[
"plan"
]
assert plan_dur > 10000.0

We need an additional test that asserts that a table request (with no resource embedding) finishes in a short plan_dur after reloading the schema cache (calling /rpc/notify_pgrst in the test).

Comment thread test/spec/Feature/Query/DeleteSpec.hs Outdated
Comment thread CHANGELOG.md Outdated
@taimoorzaeem
taimoorzaeem marked this pull request as draft January 22, 2025 15:16
@taimoorzaeem
taimoorzaeem force-pushed the non-existing-table/issue3697 branch from f008e21 to bdaf6bf Compare January 23, 2025 08:19
@taimoorzaeem
taimoorzaeem marked this pull request as ready for review January 23, 2025 08:30
@taimoorzaeem

taimoorzaeem commented Jan 24, 2025

Copy link
Copy Markdown
Member Author

One last concern, we are still using NotFound error in a couple places, which returns an empty error json:

toJSON NotFound = JSON.object []

This seems fine for now, but maybe should be improved later (add error msg, details etc) for better UX?

@taimoorzaeem
taimoorzaeem force-pushed the non-existing-table/issue3697 branch 3 times, most recently from 8d6bb71 to bb4f692 Compare February 1, 2025 07:43
@taimoorzaeem
taimoorzaeem force-pushed the non-existing-table/issue3697 branch from bb4f692 to 7196aa5 Compare February 5, 2025 05:56
@taimoorzaeem

taimoorzaeem commented Feb 7, 2025

Copy link
Copy Markdown
Member Author

@steve-chavez Could this get a review? Seems good to me.

Comment thread src/PostgREST/Error.hs Outdated
Comment thread src/PostgREST/Error.hs Outdated
Comment thread src/PostgREST/Plan.hs Outdated
@taimoorzaeem
taimoorzaeem marked this pull request as draft February 10, 2025 11:42
@taimoorzaeem
taimoorzaeem force-pushed the non-existing-table/issue3697 branch 6 times, most recently from 2b80a7c to 06532d2 Compare February 20, 2025 07:04
@taimoorzaeem
taimoorzaeem marked this pull request as ready for review February 20, 2025 07:10
Comment thread src/PostgREST/Plan.hs
Comment thread src/PostgREST/Error.hs Outdated
Comment thread src/PostgREST/Error.hs Outdated
@taimoorzaeem
taimoorzaeem force-pushed the non-existing-table/issue3697 branch from 06532d2 to 249b405 Compare February 21, 2025 06:08
@taimoorzaeem
taimoorzaeem marked this pull request as draft February 21, 2025 07:27
@taimoorzaeem
taimoorzaeem marked this pull request as ready for review February 21, 2025 10:03
@steve-chavez
steve-chavez merged commit 390ba19 into PostgREST:main Feb 21, 2025
@taimoorzaeem
taimoorzaeem deleted the non-existing-table/issue3697 branch February 22, 2025 05:46
@wolfgangwalther

Copy link
Copy Markdown
Member

I tried to backport this, but it doesn't apply cleanly, probably needs some of the refactors before that. So won't backport.

@taimoorzaeem

Copy link
Copy Markdown
Member Author

I don't think this should be backported anyways as discussed in #3869 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Insert into non-existing table raises empty APIErrror Return 404 on none existing endpoint with resource embedding

4 participants