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

Extract CREATE statements from a table #37

Open
ndmitchell opened this issue Jun 15, 2015 · 27 comments
Open

Extract CREATE statements from a table #37

ndmitchell opened this issue Jun 15, 2015 · 27 comments

Comments

@ndmitchell
Copy link

Should be pretty easy to generate a default CREATE statement.

@tomjaguarpaw
Copy link
Owner

This would be very useful. It requires some thought. When we create a table we have to tell the DBMS what types the columns have. This means we need a mapping from (type level) PGTypes to value level encodings of these. The latter don't exist yet, but I'll try to think of the most judicious way to add them.

@ryskajakub
Copy link
Contributor

One option, how to do this would be to use something similar to the Writer, with the difference in the a type, something like this:

newtype SchemaMaker read dummy = SchemaMaker (PM.PackMap ColumnDescription () read ())

the mapping from the type level to the pg types should be pretty straightforward if we don't want to have the option of specifying the range of the pg type, if we are ok with one pg type per type on haskell type level.

Or do you have something that is more elegant in mind?

@tomjaguarpaw
Copy link
Owner

I think I would prefer to have all the data you need to create the table already stored in the Table type. This is quite easily doable, but is a breaking change since will need

required :: String -> Writer (Column a) (Column a)

to change to

required :: PGType a => String -> Writer (Column a) (Column a)

for some new class PGType which knows how to convert PGText into "Text" and PGInt4 into "Int4" etc..

Your suggestion doesn't require a breaking change, however, since it only introduces new names.

@ryskajakub
Copy link
Contributor

So you mean, that the table properties would get one more field:

data TableProperties writerColumns viewColumns =
  TableProperties (Writer writerColumns viewColumns) (View viewColumns) ColumnDescription

and the information, how to create a row in a CREATE statement would be stored there.

Also, the user should be able to specify other attributes, that can't be taken from the type system like not null, that is indices, generation from sequence etc. We would probably need to add variants of required and optional with more arguments.

@tomjaguarpaw
Copy link
Owner

So you mean, that the table properties would get one more field

That's right.

Also, the user should be able to specify other attributes, that can't be taken from the type system

Yes, and probably most importantly, keys and foreign keys. However, it seems a bit awkward to do these.

@ryskajakub
Copy link
Contributor

How would that look like? I'm imagining, that the referencing of foreign key could work like runUpdate, the required would accept a Table and a function read -> Column a. Although I'm not sure, if it would work for foreign keys aiming at a column in the same table.

@tomjaguarpaw
Copy link
Owner

I think it would look something like (taking liberties a bit with the types -- this is not the full specification of Table!)

foreignKey :: Table columns -> (columns -> columnSubset)
           -> Table columns' -> (columns' -> columnSubset')

For example

a :: Table (Column PGInt4, Column PGBool, Column PGString)
b :: Table (Column PGBool, Column PGInt4)

foreignKey a (\(x, y) -> (y, x)) b id

The problem is that we have to make sure those functions can only project, not do any operations, so they can't actually work on Column. We'd have to have a newtyped thing, TableColumn maybe. It's a bit awkward.

@ryskajakub
Copy link
Contributor

Regarding the foreign keys, what about this signature:

foreignKey :: 

  (D.Default FKMaker from from' , 
  D.Default FKMaker to to'
  D.Default U.Unpackspec fk fk)

  => Table a0 from 
     -> (from' -> fk) 
     -> Table a1 to 
     -> (to' -> fk) 
     -> ForeignKey

where the FKMaker is just taking the columns and wrapping them in newtypes. It is defined in the following snippet. Also we could strip the Identity and create ProductProfunctor by hand. Moreover, the FKColumn could wrap the column name directly, not wrapped in PrimExpr, so one does not need to pattern match on the BaseTableAttrExpr.

newtype FKMaker a b = FKMaker (a -> Identity b)
newtype FKColumn a = FKColumn HPQ.PrimExpr
instance D.Default FKMaker (IC.Column a) (FKColumn a)

With regards to the column specification, what about having this:

optional :: (PGType a) => 
  Options a -> String -> TableProperties (Maybe (Column a)) (Column a)

The Options datatype further specifies the type, for example precision for numeric pg type.

class PGType a where
  data Options a
  pgTypeName :: a -> TypeName
  pgTypeOptions :: Options a -> TypeOptions

I did a basic implementation of this and it typechecks, so I'd like to hear from you if it's a way worth going or not.

@tomjaguarpaw
Copy link
Owner

This is a good start. I think it won't work because a user can apply operators to the Columns that appear in the Table before applying your foreignKey function. This can lead to invalid foreign key specifications. For example we might end up with

FOREIGN KEY (column1 + column2) REFERENCES table2 column3

which is invalid. I think what we actually need is to create tables like

table1 :: Table something (TableColumn a, TableColumn b)

Then I think your foreignKey signature would work (but it wouldn't need the FKMakers). We would need a new product profunctor for queryTable which would have to convert TableColumns to Columns.

@ryskajakub
Copy link
Contributor

At which point can the user change the Column in the Table? You mean something like:

  1. User defines a Table: table1 :: Table (Column PGInt4) (Column PGInt4)
  2. User will define a new table table1' with modified the column name by extracting table properties of table1 and fmaping on it.
  3. User calls mistakenly foreignKey on table1' and that would generate an incorrect fk.

This is the same as if the user would call the queryTable on the table1', which is currently permitted.


Anyway I also feel that it's good idea to have some TableColumn that would hold any information about the column that the user entered in the required and then make multiple extractors, one for queryTable, the other for foreignKey.

@tomjaguarpaw
Copy link
Owner

I don't understand your point about queryTable. The return value of queryTable is a Query, which one will not be allowed to generate foreign keys from.

@ryskajakub
Copy link
Contributor

I meant that it we consider the situation I explained a bad thing, that the user can somehow create a new table from an existing definition and then using it in foreignKey, then a similar bad thing happens now when the user would run a queryTable on table with altered name of some columns.

This is the only point I can see where the changing of columns in Table can happen, in the (from' -> fk) function the user is already operating on the newtypes over the Column, so it's just projection there - we would not export the FKColumn constructor so the user cannot look under the columns and modify it there.

@tomjaguarpaw
Copy link
Owner

Ah but it's not a bad thing to alter the columns of a Table and then run runQuery! Or at least it's not an SQL crash bug. It may be indeed a strange thing though.

@ryskajakub
Copy link
Contributor

So can you, please, point out, where the application of the operator before running the foreignKey would happen? Which function the user will apply , from Table to Table in order to create a Table that will create invalid foreign keys when passed in the foreignKey function?

@tomjaguarpaw
Copy link
Owner

Yeah, suppose I've got

table1 :: Table (Column Int)

What happens when I try to run foreignKey (fmap (2 *) table1) ...? I'll get something like

FOREIGN KEY (2 * column1) REFERENCES ....

@ryskajakub
Copy link
Contributor

So if the type held under View from the Internal.Table module will be effectively changed to TableColumn a, then the ColumnMaker instance needed for querying tables of would be this one:

instance Default ColumnMaker (TableColumn a) (C.Column a) where

It will just extract the name of the column and wrap it into HPQ.PrimExpr.

For the extracting the information about the columns from the table, there needs to be something similar to Writer:

newtype SchemaMaker read dummy =
  SchemaMaker (PM.PackMap ColumnDescription () read ())

where ColumnDescription is a record with the data that the user entered during the creating of table column using required or optional, that is info like column name, pg data type name, etc. That is then extracted from the table and converted to sql.

That way, the user should be indeed from fmapping the table and changing the columns to some expression. It will change the write type of all current Table definitions though.

@tomjaguarpaw
Copy link
Owner

Yes, I think your summary is correct. It is indeed a breaking change so I am a bit wary.

@ryskajakub
Copy link
Contributor

Maybe we can have these two variants together, required would create Table with Columns, whereas some other variant, for example required' would create Table with TableColumns.

There would be needed these two instances for querying the table:

 instance Default ColumnMaker (TableColumn a) (C.Column a) where
 instance Default ColumnMaker (C.Column a) (C.Column a) where

@tomjaguarpaw
Copy link
Owner

Wouldn't that mean users would have to define their tables twice?

@ryskajakub
Copy link
Contributor

No, there would exist the old variant, that just supports querying for not breaking existing user code and the new variant would support both, querying and extracting schemas.

Only users who have existing Table definitions and now want to extract the schema as well would need to rewrite the table definitions.

@tomjaguarpaw
Copy link
Owner

Sure, we can do that. Actually my biggest concern is with type inference. The type of queryTable would have to change to

queryTable :: D.Default TM.TableColumnMaker tablecolumns columns =>
              Table a tablecolumns -> Q.Query columns

and suddenly users might have to start using type annotations. That's not a good situation to be in, but I don't see a way around it.

@ryskajakub
Copy link
Contributor

Or, if we keep the queryTable as it is and make a queryTable' with the signature you wrote, then it could work. It start to feel like two libraries in one though.

@tomjaguarpaw
Copy link
Owner

At some point you have to convert TableColumns to Columns. At that point you lose type inference. It doesn't matter where it happens. It's not good.

@ryskajakub
Copy link
Contributor

I mean the type inference for users querying Table (Column a) (Column b) would work. The type inference for users querying the Table (Column a) (TableColumn b) would suffer, that's right.

@ryskajakub
Copy link
Contributor

So do you think it's worth it even though the inference would suffer? If yes then I'd like to implement it.

@tomjaguarpaw
Copy link
Owner

I think an implementation would be worthwhile anyway just so we can play around with it, but it's not something I will merge lightly. It needs a lot of thinking about and discussion with other users.

@ryskajakub
Copy link
Contributor

OK I'll try to create some draft.

ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Jul 18, 2015
@ryskajakub ryskajakub mentioned this issue Jul 18, 2015
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Jan 14, 2017
In order to be able to only project columns from Table, it's read type must not have any available functions that can modify it like Column does.
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Jan 14, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Jan 14, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Jan 14, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Jan 14, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Feb 10, 2017
In order to be able to only project columns from Table, it's read type must not have any available functions that can modify it like Column does.
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Feb 10, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Feb 10, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Feb 10, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Feb 10, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Feb 10, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Feb 11, 2017
ryskajakub added a commit to ryskajakub/haskell-opaleye that referenced this issue Feb 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants