PostgREST / postgrest

REST API for any Postgres database
https://postgrest.org
MIT License
23.48k stars 1.03k forks source link

Easier debugging of detected relationships #3752

Open steve-chavez opened 1 month ago

steve-chavez commented 1 month ago

Problem

Let's say I want to debug if there's a one-to-one relationship detected (sometimes this can be hard in the presence of views).

Using the runtime schema cache and jq it can be done by searching the table name (country) and the O2O tag:

# start postgrest
# PGRST_ADMIN_SERVER_PORT=3001 PGRST_DB_AGGREGATES_ENABLED=true postgrest-with-postgresql-16  -f test/spec/fixtures/load.sql postgrest-run

curl 'localhost:3001/schema_cache' | \
jq '.dbRelationships | .. | objects | select(.relTable.qiName == "country" and .relCardinality.tag == "O2O")'

{
  "relCardinality": {
    "isParent": true,
    "relColumns": [
      [
        "id",
        "country_id"
      ]
    ],
    "relCons": "capital_country_id_fkey",
    "tag": "O2O"
  },
  "relFTableIsView": false,
  "relForeignTable": {
    "qiName": "capital",
    "qiSchema": "test"
  },
  "relIsSelf": false,
  "relTable": {
    "qiName": "country",
    "qiSchema": "test"
  },
  "relTableIsView": false,
  "tag": "Relationship"
}

This has some issues:

Solution

A custom media type that shows all the relationships for a resource.

GET /country
Accept: application/vnd.pgrst.relationships+json
[
    {
      "relCardinality": {
        "isParent": true,
        "relColumns": [
          [
            "id",
            "country_id"
          ]
        ],
        "relCons": "capital_country_id_fkey",
        "tag": "O2O"
      },
      "relFTableIsView": false,
      "relForeignTable": {
        "qiName": "capital",
        "qiSchema": "test"
      },
      "relIsSelf": false,
      "relTable": {
        "qiName": "country",
        "qiSchema": "test"
      },
      "relTableIsView": false,
      "tag": "Relationship"
    }
]

Related

wolfgangwalther commented 1 month ago

Hm, some thoughts:

steve-chavez commented 1 month ago

Wouldn't it be much better to represent those relationships properly inside the OpenAPI output instead of with a entirely separate result?

@laurenceisla Since you've been working on OpenAPI, do you know if there's a way to represent this metadata? (whether openAPI v2 or v3)

Just changing the accepted media type should not suddenly return metadata instead of actual data on the same endpoint. The response should be represented differently, but the actual content should be the same. Not the case in your proposal.

But we do that for the the explain plan right? Overall I thought that is much more convenient to do GET /tbl + header than GET /?resource=tbl on the root endpoint, but I wouldn't be opposed to doing the latter. Specially if we add some response header that links to the resource definition when doing GET /tbl. I recall that there was a standard header about that.

We should not dump the internal types as-is, if we do something like that. Otherwise every change to those internal types will be a breaking change to our api. The admin server's schema_cache endpoint is already borderline - we should probably document that the actual response format is not safe to rely on across any version (not even patch), because it might change at any time.

Agree.

laurenceisla commented 1 month ago

@laurenceisla Since you've been working on OpenAPI, do you know if there's a way to represent this metadata? (whether openAPI v2 or v3)

One way would be to include the embedding in the schema for the content-type of Response Objects. For example, the schema could look something like:

{
  "schema": {
    "type": "array",
    "items": {
      "type": "object",
      "required": [
        "id",
        "name"
      ],
      "properties": {
        "id": {"..."},
        "name": {"..."},
        // Show the relationship here, as another property of the response object
        "child_entities": {
          "type": "object",
          "properties": {"..."}
        }
      }
    }
  }
}

Since child_entities is an "object", not an "array", then it's a to-one relationship. To check if it's a one-to-one, the schema for the /child_entities response would need to be checked as well.

Edit: If I'm not mistaken a description can also be added to the child_entities property and, additionally, mention the cardinality there.

wolfgangwalther commented 1 month ago

But we do that for the the explain plan right? Overall I thought that is much more convenient to do GET /tbl + header than GET /?resource=tbl on the root endpoint, but I wouldn't be opposed to doing the latter. Specially if we add some response header that links to the resource definition when doing GET /tbl. I recall that there was a standard header about that.

Right, we do. But the situation is different. The "query to fetch this data" is much more connected to the "data" than the relationship metadata proposed here. Think about it differently: If you add various filters, select, limit etc. to the request's URL - those would not make sense at all for the proposal here. But they do make a lot of sense for the explain output.