crystal

crystal copied to clipboard

Example:

CREATE SCHEMA s1;
CREATE SCHEMA s2;
CREATE TABLE s1.test_case(ID serial, value text);
CREATE TABLE s2.test_case(ID serial, value text);

And then start the server on the schemas s1,s2: postgraphql --connection pg://langpavel:xxx@localhost:5434/namecollision --schema s1,s2

Error: Collection of name 'test_cases' already exists in the inventory.
    at Inventory.addCollection (/home/langpavel/.nvm/versions/node/v6.9.1/lib/node_modules/postgraphql/build/interface/Inventory.js:28:19)
    at Object.addPgCatalogToInventory (/home/langpavel/.nvm/versions/node/v6.9.1/lib/node_modules/postgraphql/build/postgres/inventory/addPgCatalogToInventory.js:24:23)
    at Object.<anonymous> (/home/langpavel/.nvm/versions/node/v6.9.1/lib/node_modules/postgraphql/build/postgraphql/schema/createPostGraphQLSchema.js:53:20)
    at next (native)
    at fulfilled (/home/langpavel/.nvm/versions/node/v6.9.1/lib/node_modules/postgraphql/build/postgraphql/schema/createPostGraphQLSchema.js:4:58)
    at process._tickCallback (internal/process/next_tick.js:103:7)

In psql, behavior is driven by search_path.

For example:

INSERT INTO s1.test_case (value) VALUES ('Schema s1');
INSERT INTO s2.test_case (value) VALUES ('Schema s2');

SELECT * FROM s1.test_case; -- Schema s1
SELECT * FROM s2.test_case; -- Schema s2

-- SELECT * FROM test_case;
-- ERROR:  42P01: relation "test_case" does not exist

SET search_path=s1, s2;
SELECT * FROM test_case; -- Schema s1

SET search_path=s2, s1;
SELECT * FROM test_case; -- Schema s2

Suggested implementation

Will be more convenient if postgraphql --schema argument behaves like search_path.

There should be more options, I like only the first one:

• --hide-schema-collisions - accept only first name on search_path

• --alias-schema-collisions - excepth first name, other object will be prefixed with schema.

  For example:

  • s1.test_case becomes test_case because s1 is first in --schema argument, but
  • s2.test_case becomes _s2__test_case.

Default behavior can be like first, except it can write handy warnings into console.

As I said I don't like second option... If access to different schema is needed it can be done through view.

Hi @calebmer. Do you have any suggestions?

Yep. I like the idea of hiding collisions by default with a warning and then providing an option like --alias-schema-collisions. That was one of my initial ideas.

I’m also going to cc @mytototo from https://github.com/calebmer/postgraphql/issues/181. This may also be used for the node collision.

Was about to make an issue for this, but found this one, so would like to bring it back to life. Is there a workaround, or will it just throw errors no matter what if you have two schemas with the same name?

@calebmer is there an easy way to namespace the methods that get generated? For the example above, what about something like s1TestCase() and s2TestCase(), and just use the schema name in addition to the methods, etc. Or is it not that simple?

This could be implemented as part of #338 (Custom Naming Support) - i.e. if you know there's going to be conflicts then you have to provide a naming class which can then take the schema and table name to return a field name.

I don't like the idea of only doing this if there's a conflict - because it would result in GraphQL fields being renamed which I think is bad - so I would just always prefix the schema name or never do so. (Or never do so for the primary schema but always do so for the secondary schema, whatever works.)

In v4 you'll be able to provide custom inflection which will allow you to use both the table name and the schema name to generate the GraphQL field name.

I had the same issue. My application is multi-tenant using schema per "client". Therefore we had to use postgraphile on multiple identical schemas. In order to achieve that, we used the solution I found here: #427, running postgraphile on different route per client. Wanted to ask if it seems like a good solution for this problem, to run postgraphile per client? There is a multi-tenant plugin but it is based on column-id per "client" and not schema so we could not use it. By the way, I could not find documentation for custom inflection usage. Thanks ahead!

This isn't germain to the topic per se, but I was wondering if you had researched the use of schema / client for multi-tenant? Doing even a causal google of different ways to do multi-tenant, the per schema approach was basically downgraded as a "bad idea" due to various issues dealing with performance, maintenance, etc.

Our multi-tenant does it based on ye olde "customer-id" column type thing .. and has been working and has scaled to 1000+ clients over 10 years.

YMMV

Cheers

On Jul 31, 2018, at 11:35 AM, Koren Zerah [email protected] wrote:

I had the same issue. My application is multi-tenant using schema per "client". Therefore we had to use postgraphile on multiple identical schemas. In order to achieve that, we used the solution I found here: #427 https://github.com/graphile/postgraphile/issues/427, running postgraphile on different route per client. Wanted to ask if it seems like a good solution for this problem, to run postgraphile per client? There is a multi-tenant plugin but it is based on column-id per "client" and not schema so we could not use it. By the way, I could not find documentation for custom inflection usage. Thanks ahead!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/graphile/postgraphile/issues/191#issuecomment-409264499, or mute the thread https://github.com/notifications/unsubscribe-auth/AB8M7axokAnfqz8q2K6hWs9XFARzsx7Mks5uMHkmgaJpZM4KiEi4.

You can provide custom inflectors quite easily in v4; e.g. if you wanted to change the patch input to update mutation from *Patch to *ChangeSet you could create a plugin like this:

MyInflectorPlugin.js:

const { makeAddInflectorsPlugin } = require('graphile-utils');

module.exports = makeAddInflectorsPlugin({
  patchType(typeName: string) {
    // return this.upperCamelCase(`${typeName}-patch`);
    return this.upperCamelCase(`${typeName}-change-set`);
  },
});

Then --append-plugins `pwd`/MyInflectorPlugin.js

The built in inflectors are defined in PgBasicsPlugin; you can overwrite any that you want. Be sure to not use arrow functions otherwise you'll override this and lose access to the other inflectors.

https://github.com/graphile/graphile-build/blob/0fce134cac219d196ec97ead668ddbef7140b66c/packages/graphile-build-pg/src/plugins/PgBasicsPlugin.js#L161-L542

Let's imagine a multi-tenant application where each tenant operates within its own PG schema, and each tenant PG schema is exactly the same as every other tenant schema. Say there are 100 tenants, so the solution suggested above and elsewhere (#427) is unreasonable. We can't invoke and hold in memory 100 instances of postgraphile.

I've reviewed the inflection plugins and understand their effects on naming, but that doesn't seem to be the solution. What I want to do is change the resolver to prepend the tenant schema name, nothing more. The GraphQL request itself should not reference the tenant name in any way (i.e., two different clients can submit precisely the same request and receive only their tenant-specific results). What's the best way to go about this?

Ignore auth and security for now -- assume tenants are auth'd via JWT, and each has an individual PG role that ensures data outside the authorized PG schema cannot be accessed. It's really just about prepending the PG schema name to every referenced table.

Certainly open to writing a plugin and/or opening a PR with a bit of guidance on how to approach.

I think to do that you’d have to tell postgraphile not to use fully qualified table names, and then set the schema in the search path setting; or tell postgraphile to override the schema name when it resolves the request. The latter we could do with a forcePgSchema setting in context, then most places we reference namespaceName would change to context.forcePgSchema || namespaceName. Problem is in some cases we construct the table reference outside the resolver, so that wouldn’t work... so the search path approach is probably better.

This would require a PR to core to add a flag to do this, and a lot of tests. If you want to take this on I’m happy to give guidance but I’m really uncomfortable with the approach - there are many many ways for your database schemas to be subtly different and thus PostGraphile to fail in interesting ways. For example if the table columns are in different orders due. However it’s been requested sufficiently many times now that I think we have to support it, but it will have to come with some heavy warnings.

After further consideration, I agree with the search path approach (which is a clean solution in other related contexts). I am well aware of the advantages and limitations (as referenced in your warnings).

Are you interested in taking this on?

Please create a new clean issue for this.

@michaelbridge Just wanted to share our solution - we left the "Schema per client" approach and moved to tenantID column. With postgres it's really easy, we used row level security to ensure a user only selects the relevant rows, based on tenantID from his JWT. Feel free to contact me for more information, It's not related to this repo :)

Writing an inflection-plugin which adds the schema name as prefix for certain tables could be a solution. This is my plugin which solves the issue

const {makeAddInflectorsPlugin} = require('graphile-utils');

const MakeAddInflectorsPlugin = makeAddInflectorsPlugin({

  _tableName(table) {
    if (table.namespaceName === "some_schema") {
      return 'custom-prefix-' + (table.tags.name || table.type.tags.name || table.name);
    }
    else {
      return table.tags.name || table.type.tags.name || table.name;
    }
  },

}, true);

module.exports = MakeAddInflectorsPlugin;

After further investigation, setting search_path on a per-request basis is relatively simple:

pgSettings: req => {
    [ ... ]
    return {
        'search_path': jwtClaims['tenant'] + ', public',
    }
}

But the execution of the query itself doesn't respect the search path (presumably because the default schema name is explicitly prepended, as referenced by @benjie above). Any orienting thoughts on how to disabled "fully qualified table names" in a streamlined manner?

The second paragraph says what the approach would have to be; it sadly requires a PR to core :( https://github.com/graphile/postgraphile/issues/191#issuecomment-460556640

Agreed -- I intended to request the cleanest entry point for revising core. After further review, this would seem to be QueryBuilder.build, so as to leave the introspection mechanics untouched.

Actually I’m planning on changing that interface anyway; I wonder if I can implement this at the same time...

(At the moment it’s pretty much everywhere that references namespaceName that needs to be changed)

If you want to prefix all tables and functions with the postgresql schema name:

import { makeAddInflectorsPlugin } from "graphile-utils";
import * as lodash from "lodash";

export const MakeSchemaPrefixPlugin = makeAddInflectorsPlugin(
  {
    _functionName(proc): string {
      const funcName = proc.tags.name || proc.name;
      return proc.namespaceName + lodash.capitalize(funcName);
   },

   _tableName(table): string {
      const tableName = table.tags.name || table.type.tags.name || table.name;
      return table.namespaceName + lodash.capitalize(tableName);
    },
  },
  true,
);

I've got an identical use case to michaelbridge, and am currently running multiple postgraphile instances - one per tenant. The approach works but I'm needing to find an alternative now as this approach won't scale, and my number of tenants is growing. The search_path solution would be ideal - just wondering if there is any update on this?

Work on V5 has begun; but it will probably be a few months before it's out.

Hi, i am beginner in postgrphile, can any one get me a solution to list all tables and views.

@Chandra-mishra It's not clear what you mean; please ask in chat so we can guide you more interactively: https://discord.gg/graphile

i want to list all tables which are available in database.

@benjie i want to list all tables which are available in database.

@Chandra-mishra your question seems like a postgres specific question, not exactly a postgraphile issue, or even related to this issue specifically which is regards to naming collisions. This issue is completely different that what I understand your issue to be. I'd advise you to go into the discord and ask there, and if I do understand your question, which sounds more postgres related, you can find many answers including this one on stackoverflow: https://dba.stackexchange.com/questions/1285/how-do-i-list-all-databases-and-tables-using-psql

Is there any news on this?

I'm not sure why my comment was marked as spam... I have the exact same use case as @michaelbridge and I wanted to know if this is still planned to be possible in v5

Comments asking for updates that are unclear on the request (you can probably see there’s a lot of off-topic/misinterpretation in this thread, so the “this” was definitely unclear) and add nothing to the conversation consume maintainer time that could be spent actually implementing the features desired. They’re a very common form of open source issue spam (along with comments like “+1”, which thankfully seems to be dying off).

Thank you for clarifying your request. All updates on this specific feature are in the above thread. It’s still slated for v5, and work on v5 is ongoing (in private) - in fact I’m scheduled to work on v5 today once I’m caught up on GitHub notifications.