volatiletech / sqlboiler

Generate a Go ORM tailored to your database schema.
BSD 3-Clause "New" or "Revised" License
6.66k stars 539 forks source link

SQLBoiler is not cross-schema-aware. Need qualified table identifiers for FK. #134

Open FourSigma opened 7 years ago

FourSigma commented 7 years ago

SQL Boiler Version 2.20 using Postgres 9.6

When referencing a foreign key from another schema, SQLBoiler panics.

CREATE SCHEMA IF NOT EXISTS identity;
CREATE TABLE identity.profile ( 
       id         uuid PRIMARY KEY
)

CREATE SCHEMA IF NOT EXISTS account;
CREATE TABLE account.user(
       profile_id         uuid UNIQUE REFERENCES identity.profile ON DELETE CASCADE
)

Executing the command:

╰─$ sqlboiler -s account postgres
panic: could not find table name: profile

goroutine 1 [running]:
github.com/vattle/sqlboiler/bdb.GetTable(0xc420392000, 0x3, 0x4, 0xc420131680, 0x7, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /Users/siva/go/src/github.com/vattle/sqlboiler/bdb/table.go:30 +0x22c
github.com/vattle/sqlboiler/bdb.setForeignKeyConstraints(0xc420392090, 0xc420392000, 0x3, 0x4)
    /Users/siva/go/src/github.com/vattle/sqlboiler/bdb/interface.go:115 +0x173
github.com/vattle/sqlboiler/bdb.Tables(0x18d71c0, 0xc420334660, 0x7fff5fbffa2d, 0x7, 0x1927a08, 0x0, 0x0, 0x1927a08, 0x0, 0x0, ...)
    /Users/siva/go/src/github.com/vattle/sqlboiler/bdb/interface.go:78 +0x98d
github.com/vattle/sqlboiler/boilingcore.(*State).initTables(0xc420364000, 0x7fff5fbffa2d, 0x7, 0x1927a08, 0x0, 0x0, 0x1927a08, 0x0, 0x0, 0xc4200f3820, ...)
    /Users/siva/go/src/github.com/vattle/sqlboiler/boilingcore/boilingcore.go:336 +0xb5
github.com/vattle/sqlboiler/boilingcore.New(0xc420354000, 0x0, 0x0, 0x5)
    /Users/siva/go/src/github.com/vattle/sqlboiler/boilingcore/boilingcore.go:66 +0x123
main.preRun(0xc42033c000, 0xc420331380, 0x1, 0x3, 0x0, 0x0)
    /Users/siva/go/src/github.com/vattle/sqlboiler/main.go:296 +0x54e
github.com/spf13/cobra.(*Command).execute(0xc42033c000, 0xc420010250, 0x3, 0x3, 0xc42033c000, 0xc420010250)
    /Users/siva/go/src/github.com/spf13/cobra/command.go:623 +0x487
github.com/spf13/cobra.(*Command).ExecuteC(0xc42033c000, 0xc42033e000, 0x0, 0x0)
    /Users/siva/go/src/github.com/spf13/cobra/command.go:710 +0x339
github.com/spf13/cobra.(*Command).Execute(0xc42033c000, 0x0, 0x0)
    /Users/siva/go/src/github.com/spf13/cobra/command.go:669 +0x2b
main.main()
    /Users/siva/go/src/github.com/vattle/sqlboiler/main.go:104 +0xd1a
aarondl commented 7 years ago

Hey @FourSigma, thanks for reporting the issue. SQLBoiler is schema-aware of course since you're using the --schema argument :p But obviously the implementation wasn't ready for cross-schema anything. The intention definitely was that each schema is self-contained, and each schema can be generated and used separately. Hadn't imagined this scenario (frankly didn't even know it was possible in pg).

We do schema prefix most things in the generated models, so I wonder if all we'd need to do is fix the driver so it wouldn't bomb on loading up the data properly. It'd probably also require a ton of extra "Schema" pieces on all the data structures. I'll have to look at it when I have a bit more time on my hands to really qualify it.

FourSigma commented 7 years ago

Thanks for the response! hahha...i should have choose my words more carefully. I really enjoy using this library for personal projects. At work we do a lot of cross-schema references in pg (we use a custom sql generator). We mostly use schemas to namespace similar concepts. I would love to help out with this project, do you know what might be the best place to start? I have a couple of ideas that I want to investigate.

aarondl commented 7 years ago

The code takes into account a schema that comes from here: https://github.com/vattle/sqlboiler/blob/master/boilingcore/config.go#L6 Obviously this is a global schema and has nothing to do with the table/columns, it was more of a switch to say "which schema are we generating code for".

That schema is used all over the place to prefix tables to make sure it works when we've generated for a schema that isn't 'public'. https://github.com/vattle/sqlboiler/blob/master/templates/09_relationship_to_many_eager.tpl#L40 https://github.com/vattle/sqlboiler/blob/master/templates/15_insert.tpl#L69

But it's never used to prefix columns, and tables of a different schema were never anticipated. The reason we went this "global schema" route is because it seemed at the time like a good solution and it meshed well with MySQL.

I think in order to truly fix this, each thing inside the bdb/{tables.go,keys.go} would have to contain schema information, which then the bdb/drivers/{postgres,mysql,mssql} would then have to set properly. And then whenever we reference tables or foreign keys in all the templates we'd have to include the schema.

You'd also probably need the ability to specify multiple schemas without which this feature wouldn't work. Since the schema currently determines which set of tables to look at, so you'd have to be able to say: do schemas X, Y, and Z. And any cross reference stuff wouldn't matter.

Now, if you were talking about other ideas you wanted to investigate in terms of you have other ideas for SQLBoiler, I'd be super happy to discuss them with you and see if we can't fit them in :)

FourSigma commented 7 years ago

Thanks for the detailed info. Implementing cross-schema awareness looks like a major change. I have a couple of other smaller ideas orthogonal to this one which will help me get a bit more comfortable with the code base. I will shoot some pull requests when I get some free time.

aarondl commented 7 years ago

Yeah. Looking at this and talking about it with @nullbio. This is actually a humungous change as it invalidates several assumptions. The entire model of MSSQL/MySQL is built around this too. It's an earth-shatterer :(

orian commented 6 years ago

The hack I've used (it's ugly) is to drop foreign key constraints for a time of sqlboiler run.

Edit: the main reason of using PG schemas is to separate table (db object) spaces. In our architecture "plugins" have their own schemas and do not reside inside the main schema. Makes a db more manageable.

aarondl commented 6 years ago

Good to know :)

saulortega commented 6 years ago

I think the best way to preceed with this is:

  1. Optional support with a flag.
  2. Queries must be schema-dependent and they must be generated dinamicaly on runtime: models.Users().All("my_schema", db) or models.Users(qm.Schema("my_schema")).All(db). This allows the use of user-dependent schemas.

I just put this comment here.

Maybe for version 4. I will use multiple database for my upcoming project.

aarondl commented 6 years ago

@saulortega Note that multiple database and multiple schema is okay. So long as it's not cross schema, meaning abc.table has a column that refers to xyz.table.row in a foreign key, or something like that. If schemas are self-contained, and used in isolation as far as the database is concerned there is no problem here at all. You just generate two different models packages.

c3mb0 commented 4 years ago

How about a multi-tenant system with 5000 tenants, all having the exact same schema? Are we expected to build 5000 different model packages? Seems redundant for a problem that could be solved simply by prefixing the generated table name with the provided schema name during runtime.

aarondl commented 4 years ago

Hello @c3mb0. You have an excellent understanding of the limitations and the workarounds involved. Your assertions about redundancy are also well placed and correct.

c3mb0 commented 4 years ago

In this case, is it something you are planning to address and/or would accept a PR for?

aarondl commented 4 years ago

Just to set expectations: sqlboiler is in maintenance mode as far as I'm concerned. That means no feature will be added unless the community provides it via PR. Though I do make an effort to fix many of the bugs myself. I'd accept a PR for the feature you're talking about and/or the feature this issue is talking about. To be specific the issues at hand are: Parameterizing schema/dbname at runtime vs cross-schema awareness/use.

notech commented 1 year ago

did a multi-tenant system use a single postgresql schema? how about use "SET search_path"?