Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add constraint information to generated interfaces #77

Open
frankpf opened this issue Dec 21, 2017 · 7 comments
Open

Add constraint information to generated interfaces #77

frankpf opened this issue Dec 21, 2017 · 7 comments

Comments

@frankpf
Copy link

frankpf commented Dec 21, 2017

First of all, thanks for writing this library, it is great.

I'm trying to write a type-safe SQL query builder and having information on the row constraints would be useful.

As an example, this table:

field type constraint
id text NOT NULL PRIMARY KEY
email text NOT NULL UNIQUE
pwd_hash char(60) NOT NULL
deleted boolean NOT NULL DEFAULT false
name text

could generate something like this:

export interface users {
   id: { type: string, primary: true }
   email: { type: string, unique: true }
   pwd_hash: { type: string }
   deleted: { type: boolean,  default: false }
   name: { type: string | null }
}

I understand that this feature would be overkill to most users, so it could be hidden behind a flag --mode verbose or something like that.

If there's interest in this feature, I'd be willing to work on it.

@frankpf frankpf changed the title Add constraints to generated interfaces Add constraint information to generated interfaces Dec 21, 2017
@abenhamdine
Copy link
Contributor

Good idea IMHO, could be also a way to implement #12 (comment)

@xiamx
Copy link
Contributor

xiamx commented Dec 22, 2017

Sounds good!

My only concern is that the new interface generation will not be compatible with the current way we typecheck query returns (see example https://github.com/SweetIQ/schemats#writing-code-with-typed-schema)

Maybe these information can take a different key, so instead we have two alternatives:

export interface users {
   id: string
   _id_meta: {primary: true}
   email: string
   _email_meta: {unique: true, comment: 'somecomment'}
}
export interface users {
   id: string
   email: string
}

export interface usersMeta {
   id_meta: {primary: true}
   email_meta: {unique: true, comment: 'somecomment'}
}

@frankpf
Copy link
Author

frankpf commented Dec 22, 2017

One problem with that is that it becomes more inconvenient for libraries to access the relevant keys.

As an example, I want to add a method findOne to SafeQL that accepts only primary/unique keys as arguments. Using the scheme above, the user would have to pass "_id_meta" as an argument instead of "id".

Here's what I suggest:

type HasTypeKey<T> = {
    [K in keyof T]: {
        type: any
    }
}
type SimpleSchema<T extends HasTypeKey<T>> = {
    [K in keyof T] : T[K]['type']
}

export interface usersMeta {
   id: { type: string, primary: true }
   email: { type: string, unique: true }
}

export type users = SimpleSchema<usersMeta>

You can check it on TS playground and confirm that it still type-checks correctly.

Essentially, only the "verbose" interfaces are generated (containing all the necessary information), and we use mapped types to transform them to the basic interfaces.

This would keep the library compatible with the current API and give library authors full access to the type information.

The flag --mode verbose could simply be used to remove the helper types from the output and generate the same interfaces without the "Meta" suffix.

@frankpf
Copy link
Author

frankpf commented Dec 22, 2017

@abenhamdine Yes, IMO a good design would be to output as much information as possible (for library authors to use) while keeping the types simple for end users (using the suggestion above).

What do you think of something like this:

export interface usersMeta {
   id: { type: string, primary: true, originalType: 'text' },
   pwd_hash: { type: string, originalType: 'char(60)' }
}

Although I don't know if simple strings are the best representations for the SQL types. An alternative would be:

originalType: { type: 'text' }
originalType: { type: 'char', length: 60 }

The same goes for check constraints. I see two options:

check: 'price > 0'

vs

check: { price: { greaterThan: 0 } }

Option 2 seems better, although it would be harder to implement.

@xiamx
Copy link
Contributor

xiamx commented Dec 22, 2017

Here's what I suggest:

type HasTypeKey<T> = {
    [K in keyof T]: {
        type: any
    }
}
type SimpleSchema<T extends HasTypeKey<T>> = {
    [K in keyof T] : T[K]['type']
}

export interface usersMeta {
   id: { type: string, primary: true }
   email: { type: string, unique: true }
}

export type users = SimpleSchema<usersMeta>

Nice! This is a very clean solution! With this solution, we can generate the table column type interfaces along with the metadata like primary and unique. The meta interfaces like usersMeta can be unexported by default; and is only exported with --mode verbose is passed in. This will ensure that the default behaviour stays consistent for the end user.

check: { price: { greaterThan: 0 } }

Looks like option 2 would require some facility to parse SQL DDL. This will surely take more time to implement. IMO, it would be fine to leave the parsing of SQL types to downstream libraries, since we are returning valid SQL types as string.

@frankpf
Copy link
Author

frankpf commented Feb 9, 2018

I started working on this feature on a fork.

Sadly, Postgres doesn't appear to expose constraint information in an easy-to-use manner. After asking on #postgresql and googling around a bit, I stumbled upon this: https://gist.github.com/PickledDragon/dd41f4e72b428175354d

With a few changes, this can output the constraint type (PRIMARY KEY, UNIQUE, FOREIGN KEY or CHECK). Getting the actual CHECK string (e.g. price > 0) is a bit more complicated, I'll have to look into that later.

Types like char(60) are very easy to separate into max length and data type; Postgres exposes this information in the information_schema.columns table.

One caveat is that this feature will only work on PostgreSQL (at least for now) as I don't have enough familiarity with MySQL to implement it there.

@xiamx
Copy link
Contributor

xiamx commented Feb 9, 2018

Yeah looks like not everything is easily accessible. Not having MySQL support for now would be fine, someone interested in it will eventually make a PR :D

Feel free to open a pull request when you are ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants