multiple schemas in one instance

[ghost]

No description provided.

[steve-chavez]

My bad, just merged #1435 and it caused some conflicts. Could you rebase?

Also see my comments above.

[steve-chavez]

Hmm.. the merge commits makes the PR a bit hard to review for me(change to the configSchema type to a [Text] not showing on github diff). When you think it's ready for review, please try to rebase(git rebase -i master) the branch.

(another choice maybe would be to cherry-pick each commit you made in a new branch started from upstream/master and then rename this new branch to master, finally push origin -f)

[ghost]

As for when I would say this is ready, there are 2 more things to do form what I can tell. First is add the Open API tests and second implement a check that forbids cross schema queries to go through unless both source and target schema are explicitly added in the db-schema parameter(and a test for that as well). Once I have these I can clean up to exclude the merge commits.

[steve-chavez]

@MahmoudKassem Would you like me to help you with the OpenAPI tests? I think if you check Allow edits from maintainers in the PR I can push to this branch.

[ghost]

@steve-chavez Sorry didn't have any time to work on this(new job, new apartment and some more stuff). Things are currently cooling down. I should be able to resume working on this during this week.

The tests should be fine, one for the first test schema as default and one where it is passed explicitly in the header and maybe add one more table to the second test schema to distinguish them and one more 404 test with an unknown schema. What is unclear to me is how to restrict cross schema access in the q parameter to schemas in the db-schema parameter and fail the request otherwise.

The "Allow edits from maintainers" checkbox was already set. Support especially on the second point would be much appreciated.

[steve-chavez]

I've noted that taking the head of the schemas would make the procs only work for a single schema(not the whole list). This is because we obtain some info about procs in DbStructure.

postgrest/src/PostgREST/DbStructure.hs

Lines 47 to 55 in b75e7ce

	getDbStructure :: Schema -> PgVersion -> HT.Transaction DbStructure
	getDbStructure schema pgVer = do
	HT.sql "set local schema ''" -- for getting the fully qualified name(schema.name) of every db object
	tabs <- HT.statement () allTables
	cols <- HT.statement schema $ allColumns tabs
	srcCols <- HT.statement schema $ allSourceColumns cols pgVer
	m2oRels <- HT.statement () $ allM2ORels tabs cols
	keys <- HT.statement () $ allPrimaryKeys tabs
	procs <- HT.statement schema allProcs

We would need to pass the whole list here and change some of the DbStructure queries. I can help with this.

[steve-chavez]

(new job, new apartment and some more stuff)

@MahmoudKassem Congrats!

I can help you with the part you mentioned. Also I've noted there is other stuff we need to modify(DbStructure).

I've excluded the merges on this branch and cherry-picked your commits on https://github.com/steve-chavez/postgrest/commits/mult-schemas. Let me know if that branch is ok and I'll force push it here so we can continue working on this.

[ghost]

@steve-chavez looks good all non-merge commits are there.

[steve-chavez]

@MahmoudKassem Sorry, I made a mistake when force pushing here 😞 .

I did git push mahmoud master -f instead of git push mahmoud mahmoud:master -f 🤦‍♂️. And now I can't push to your master branch.

Edit: I've opened #1450 and I've also granted you access to my fork, we can continue working there(on https://github.com/steve-chavez/postgrest/tree/multiple-schemas).