Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

sqlite: open the database as main #126

Closed
wants to merge 1 commit into from

Conversation

cryslith
Copy link
Contributor

@cryslith cryslith commented Apr 30, 2020

This allows referring to possibly-nonexistent tables with their unqualified names in raw commands, and is more consistent with the behavior of the Postgres and MySQL backends.

In particular, it enables creating tables using the Queryable interface. For instance, one can write db.cmd_raw("CREATE TABLE my_table (x INTEGER);") to create a table, instead of db.cmd_raw("CREATE TABLE db_name.my_table (x INTEGER);") (where db_name is the value of the db_name parameter in the database URL). This is useful because without it, one would have to re-parse the original database URL in order to get the value of db_name, and then thread that value through all code that needs to create a table.

For backwards compatibility, the database file is still attached as db_name too, so it's mapped as both main and db_name.

This allows referring to possibly-nonexistent tables
with their unqualified names in raw commands.
@tomhoule
Copy link
Contributor

tomhoule commented May 7, 2020

We have a fairly large codebase relying on quaint at https://github.com/prisma/prisma-engines

I think the rationale for the change is solid. We could test prisma with this change and merge if we don't find any issue, wdyt @pimeys ?

@pimeys
Copy link
Contributor

pimeys commented May 7, 2020

Yeah. We basically always have the database indicator in all our queries. Model is User and the Schema is Blog, so all the queries will be User.Blog, and user.db is attached as User.

Might be a trickier change.

@tomhoule
Copy link
Contributor

Sorry for neglecting this PR for so long. So I did try this patch on the prisma repo, and it seems like the queries on the ATTACHed database are now read-only, which is a problem since we do add the attached name everywhere. I think the way forward would be for us to no longer do that, and we can merge this PR. What do you think @pimeys ?

@pimeys
Copy link
Contributor

pimeys commented Aug 17, 2020

Yes.

@tomhoule
Copy link
Contributor

@cryslith sorry it got delayed so long. We did some testing and it turned out to be more complicated than expected on our end, but I think we should merge your PR now. It just needs to be rebased first.

@pimeys Here's the related PR: prisma/prisma-engines#1156 — there is some cleanup work left to do, but it demonstrates that we can make it work.

@cryslith
Copy link
Contributor Author

Closing since #192 was merged

@cryslith cryslith closed this Oct 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants