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
Partitioned tables are not supported by everything related to schema cache #1783
Comments
Can you give us a bit more information in the form of a self-contained example? I think you can remove the |
@wolfgangwalther sorry, I've copy-pasted a piece of code to simply show how i've setted partitions. Environment
Steps to reproduce:
DROP TABLE IF EXISTS public.test_a CASCADE;
CREATE TABLE public.test_a(
id1 int NOT NULL,
id2 int NOT NULL,
a VARCHAR(64) NOT NULL,
PRIMARY KEY (id1, id2)
) PARTITION BY LIST (id1);
DROP TABLE IF EXISTS public.test_b CASCADE;
CREATE TABLE public.test_b(
id1 int NOT NULL,
id2 int NOT NULL,
b VARCHAR(64) NOT NULL,
PRIMARY KEY (id1, id2)
) PARTITION BY LIST (id1) ;
DROP TABLE IF EXISTS public.test_lnk CASCADE;
CREATE TABLE public.test_lnk (
a1 INT NOT NULL,
a2 INT NOT NULL,
b1 INT NOT NULL,
b2 INT NOT NULL,
CONSTRAINT fk_test_a FOREIGN KEY (a1,a2) REFERENCES public.test_a(id1,id2),
CONSTRAINT fk_test_b FOREIGN KEY (b1,b2) REFERENCES public.test_b(id1,id2)
) PARTITION BY LIST (a1);
Status: {
"message": "Could not find foreign keys between these entities. No relationship found between test_a and test_b"
} Even the following queries don't work:
Also tried after created partitions on all tables and insert some values
|
This is not only related to this specific case of embedding. I wanted to take a look at this, but already noticed that the tables in your example do not even show up in the openapi schema at the root endpoint. This is because none of the queries in postgrest/src/PostgREST/DbStructure.hs Line 270 in 9f074ce
postgrest/src/PostgREST/DbStructure.hs Line 291 in 9f074ce
postgrest/src/PostgREST/DbStructure.hs Line 411 in 9f074ce
postgrest/src/PostgREST/DbStructure.hs Line 423 in 9f074ce
postgrest/src/PostgREST/DbStructure.hs Line 461 in 9f074ce
postgrest/src/PostgREST/DbStructure.hs Line 519 in 9f074ce
postgrest/src/PostgREST/DbStructure.hs Line 618 in 9f074ce
postgrest/src/PostgREST/DbStructure.hs Line 655 in 9f074ce
|
Hello team. Any thoughts on this bug? We also have the same issue with embedded resources and partitions. |
This shouldn't be too hard to implement. It will be mainly about adding some test cases and adding the relevant relkinds to the queries. Not even much haskell involved. Do you want to give it a try? |
Seems like just adding the partitioned tables to the schema cache is not giving us 100% support for partitioned tables, yet. There's a report at #1903 (comment) which says M2M embedding with partitioned tables does not work, yet. Still need to confirm, but let's reopen this in the meantime. |
There's also another report here: #1903 (comment) Looks like there might be an issue with creating duplicated entries in the schema cache in some kind of schema. @hanumanth-mv can you share a code example reproducing the problem you've reported? |
Here are the test scenario results using the Film/Actor/Director example provided on the PostgREST documentation website under Resource Embedded section. Below is the SQL Script to generate the Film tables. Tested Scenarios:
Work Around for Partitioned Table 2nd Level & Above Relationship ErrorYou can chain the embedded queries to get 2nd level and higher relationship data. For example, the 2nd level query that failed above will work if the "role" table which has a direct relationship to films is used as a bridge to get the the "actor" table
|
@BakerHughesCharlesSewell Thanks for the SQL script!
This request {
"hint": "By following the 'details' key, disambiguate the request by changing the url to /origin?select=relationship(*) or /origin?select=target!relationship(*)",
"details": [
{
"relationship": "api.role[fk_film_role][fk_role_actor]",
"origin": "api.film",
"cardinality": "m2m",
"target": "api.actor"
},
{
"relationship": "api.role_usa[fk_film_role][fk_role_actor]",
"origin": "api.film",
"cardinality": "m2m",
"target": "api.actor"
},
{
"relationship": "api.role_default[fk_film_role][fk_role_actor]",
"origin": "api.film",
"cardinality": "m2m",
"target": "api.actor"
}
],
"message": "More than one relationship was found for film and actor"
} Which means that PostgREST can reach the This means that you could also use the partitions, like Let us know if that solves the issue. |
@laurenceisla - The existence of table partitions should be hidden. When PostgreSQL creates the partitioned Role table, it automatically generates foreign keys directly to the partitioned tables (see image below) which is apparently confusing the Embedded Resource PostgREST query with too many paths to follow. As a work around, the following approaches can be followed:
A better approach would be to ignore foreign keys for partitioned tables and only cache the foreign key relationships for the primary table. Ideally, the same PostgREST query should work for both partitioned and non-partitioned tables. |
I agree. Two approaches:
The second approach is a bit more complicated, but probably what everyone would expect. |
Ah, I understand the issue now.
I agree on this approach. Unless the partitions shouldn't be used in embedding at all, I think this should be the go to. |
I thought about that, too - but there could also be a case where only a partition is in the exposed table and not the partitioned table. Or where you have a view based on a partition etc. - so we should still allow to explicitly specify partitions. |
Partitions are typically created when your table is extremely large to virtually divide the table into smaller segments to improve query performance. As a best practice, we do not expose the underlying partitioned tables since table security policies are defined at the parent and not the individual partitions. While someone might consider a View targeting LIST or RANGE partitions, this makes no sense for HASH partitions. While it is technically possible to create a View that directly queries a partition table, this is a brittle design since the partition can be dropped or renamed and is completely unnecessary since you can always have the view query the parent table with the partition key value as an input to the View. In my opinion, the default behavior would be to prune off the foreign keys for the partitioned tables to avoid the need for the odd hint nomenclature. You could add a startup parameter in the PostgREST config file to control this behavior so that someone could retain those foreign keys and leverage the hint methodology. |
Following this logic, it would make most sense to just completely ignore partitions everywhere. In the schema cache, in the OpenApi output - and certainly for embedding relationships, too. Should also be simpler to do. It is a breaking change. Although there is a workaround, by just creating views to the parent table that selects by partition key. |
Hm, this approach sounds similar to how we do self-joins(#1643 (comment)) now, which are a special case that we want to get rid off. I'd say we should avoid adding more special cases, these complicate the codebase and are also hard to document(inconsistent with the feature set and complicated for the user).
Agree. We could add a config param as Charles suggested to avoid the breaking change but I think ignoring the partitions is the right behavior. We can take advantage of the new major version to do this breaking change now. In regards to a config param, maybe it could be like: db-detect-relationships-on = "tables,views" # default
db-detect-relationships-on = "tables,views,partitions" # custom
db-detect-relationships-on = "none" # basically disabling embedding Some users only expose functions through PostgREST so they don't really need embedding, that's where the |
Hm. I feel like we don't need to add config options for everything. Every config option adds overhead (code complexity, ...). We should make sure to have config options for those things that really matter. I feel the same way about Not including partitions in the embedding just makes sense. Plus there is any easy workaround if you really need to - create views. |
Yeah, I agree. Maybe I was too quick to add that config, I see users already preferring to use |
Agreed! |
Environment
postgres:13.2
postgrest/postgrest:nightly-2021-03-05-19-03-d3a8b5f
Description of issue
This issue is related #1592 (and the fix: #1593)
I've tested the last the nightly builds and I still have the same issue, but only with partitioned tables.
It seem that PostgREST don't recognize foreign keys if the tables are partitioned, but if I remove the partitions all work fine.
The text was updated successfully, but these errors were encountered: