Add Table Updating DAO

[charJe]

Preface: I do not believe this is a complete pull request. It is missing updates to the readme (I'm happy to do that if you like the idea), I don't think the test is working right, and I am probably missing some corner cases. That said, I think this is a really useful feature and I would appreciate someone taking a look at it. Maybe with a little work, it can get merged in.

This is a functionality like dao-table-definition where is takes a dao and produces a database table. The added feature that this table updating dao adds is being able to update an existing database table. Add or remove a slot to your dao? dao-table-update will add the column in the database. Change a column type, nullability, or key? dao-table-update has you covered (unless you pick an incompatible type).

This helps keep your dao definition and the equivalent table in the database image the same without having to open up the database shell to do ALTER TABLE statements.

[sabracrolleton]

A couple of questions to start the conversation.

1. What is the useful difference between new function columns-description and existing function table-description?
2. When you have data in the table, what happens when you rename a column?

[charJe]

1. I was originally using table-description, but types it was returning were unsatisfactory: int4 instead of integer or serial, varchar instead of varchar(10), and bool instead of boolean. I really have no idea why this is or if it is intended. I didn't want to break anything, so I just made a new function.

2. Currently, it will try to convert by USING column-name::new-type. If you are converting a column from varchar(100) to text, it should work just fine. However, if you are converting something incompatible like varchar(10) to integer, the database will throw an error (that bubbles up to a lisp error). I'm not sure what the desired behavior here is because I want to change the type to match dao, but I also don't want to ruin any existing data.

[sabracrolleton]

1. int4, int8, int2 etc are specifications to Postgresql about the allowed size of the integer. Not relevant to CL, but definitely relevant to Postgresql.

2. I think you misunderstood my question. I am not asking about changing the data type. I am asking about renaming a column. For example changing a column named "zipcode" to "post_code". As I understand your code, it would determine that the "zipcode" column no longer exists, so it drops the "zipcode" column and creates a new "post_code" column. Now you have lost all the data originally in the "zipcode" column.

[charJe]

You are indeed correct. I am not sure how to detect a column name change versus adding and removing a column at the same time. It definitely would be ideal to do so though.

[svantevonerichsen6906]

This is the topic of schema migration, for which several tools exist, and which is quite hairy to get right. I fear that an incomplete solution might do more harm than good.

[sabracrolleton]

Savantevonerichsen6906: Exactly. I just tried giving the simplest example of one of the problems.

[svantevonerichsen6906]

Sorry, I didn't intend to spoil your socratic approach ;-)

[sabracrolleton]

;) Appreciate your joining the conversation. So much sounds clear and easy until there is actual data or more than one developer. Or grumpy DBAs like me.

[charJe]

Thank you both for taking interest. I agree that accidentally dropping columns is a problem. The only solution I can think of is to add another slot attribute (a slot-slot?) that would represent the sql name of the column (kebab-case of course) if this slot-slot is absent, it will take the actual slot name like it currently does. If the programmer wants to drop column and add column then rename the slot. If the programmer wants to rename the column (in the database) then change the :col-name slot-slot. How it might look:

(defclass test () 
  ((id :col-name id :col-type serial :initarg :id :accessor id)) 
  (:metaclass dao-class) 
  (:table-name dao-test) 
  (:keys id))

to rename the id column in the database:

(defclass test ()
  ((id :col-name id-test :col-type serial :initarg :id :accessor id))
  (:metaclass dao-class)
  (:table-name dao-test) 
  (:keys id))

Alternatively, to drop id column and add a new column

(defclass test () 
  ((totally-not-id :col-name totally-not-id :col-type serial :initarg :id :accessor id)) 
  (:metaclass dao-class) 
  (:table-name dao-test) 
  (:keys totally-not-id))

CLOS itself has this same problem (I think). If you change the slot name at run time any existing objects will loose their data for that slot. If you want to just change the name, but retain the data then change the :accessor, :reader, :initarg, and :writer slot-slots. As I am looking over this I realize that the :col-name slot-slot would have the exact same purpose as the :table-name meta-slot.

Would this be acceptable? I'll be happy to implement it if you give me the green light.

[sabracrolleton]

I have now recovered from my fit of apoplexy. You are right that instantiated CLOS classes will lose data if you redefine their slots. CLOS and relational databases are not the same. Databases do not lose data because you change the name of a column. Dealing with a relational database via CLOS ORM does not mean that the redefinition limitations of CLOS should ever impact the integrity of the data in a relational database.

As svantevonerichsen6906 said, schema migration is a hairy problem. Changing the name of a column is only one of the many concerns. I just thought it was the easiest one to explain. Other examples just off the top of my head would be columns that are indexed or may be part of multi-column indexes or used by stored procedures or you are splitting tables (e.g. changing a column that contains an array of tags into a separate many-to-many table) or changing foreign keys or dealing with columns that have triggers or are hooked into application code in some manner ..or ..or any of the other complexities that cause real migration tools to generate scripts that are signed off by all team members.

If you are in a development stage without real data and you want to modify a dao class, it might be easier just to drop tables and execute (dao-table-definition your-class-name) to recreate the tables from scratch, obviously checking all of this into your version management system. It is cleaner and more obvious.

I appreciate the suggestion, but I am extremely loathe to accept partial solutions when real data might exist in the database.

[charJe]

I am aware that this pull request is in an incomplete state; I just wanted to have some conversation, fix incomplete parts, and then, maybe, merge. I have no ill intent; I just think this is a useful feature that could have some practical application. Sure CLOS isn't 1-1 with databases, but things like :table-name and :col-name can smooth overs those differences.

dao-table-update was never intended as a full migration tool. For complex migrations, one would still have to jump in to SQL the same way you have to if you want a complex query. I do think that renaming tables and columns fit within the scope of dao-table-update. Currently, dao-table-update totally ignores foreign-keys. This would certainly need to be sorted out before getting merged. There is nothing forcing a team to execute the SQL generated by dao-table-update, or using it at all. Maybe a team wants to run it, then execute the generated statements only after review.

If you are in a development stage without real data and you want to modify a dao class, it might be easier just to drop tables and execute (dao-table-definition your-class-name) to recreate the tables from scratch

Sure, but the development state never stops for some projects. If I'm developing with a test database and need to add a column or change a type, then it would be better to update the table based on the DAO class rather than having to update the DAO class and separately construct a query to alter the table.

[sabracrolleton]

No ill intent assumed. As a DBA I have a tendency to react strongly when something threatens data and renaming columns in many "simple" ORMs is one of those situations. I will continue to think about the idea (I did not close it).

[svantevonerichsen6906]

CLOS already has a mechanism for migration: the user can implement methods for update-instance-for-redefined-class. I am inclined to say that if a postmodern user has a migration task that they think can be automatically resolved, they can use that.

I still think that providing a default behaviour that potentially deletes data is dangerous, and that the convenience provided by such a default is not that important in comparison.

However, maybe this can nevertheless be solved at the library level by requiring user inteaction. Here is a little draft:

• a schema-change-required-condition which is signalled by an update-instance-for-redefined-class for dao-classes,
• which carries a schema update proposal to the user
• who can then use the restart accept-proposed-schema-change
• or use-different-schema-change and enter a new expression (possibly with some support, e. g. warnings about deleted columns, or columns with different types)
• or abort, so that the class definition and db schema are not in sync anymore.

Abort would presumably lead to other errors that can only be resolved by manually changing the db schema, which is the current situation if I understand correctly. It might then be nice if the above migration support is also user-invokable without another class redefinition, but the information about which slots are added and removed would then need to be preserved somehow.

The schema update proposal could be provided by a generic function propose-schema-change-for-updated-dao-class that could be implemented by the user or other tools outside of the postmodern library itself.

I'm not sure if this leads to a happy place, but this way, the hairy parts could maybe be kept separate.

[charJe]

that could be implemented by the user or other tools outside of the postmodern library itself

I think, for now at least, I will continue my work on this in a library external to postmodern.

[sabracrolleton]

Good luck. Looking forward to seeing it when you publish.