search

kadena-io / chainweb-data

Data ingestion for Chainweb.
BSD 3-Clause "New" or "Revised" License
14 stars 8 forks source link

Constraints on Added Tables/Columns (Outside of Chainweb-data rquired) throws error #145

Closed mbwmbw1337 closed 1 year ago

mbwmbw1337 commented 1 year ago

While ORM code has been updated to allow for additional tables within the database, adding a primary key to the table or other constraints to tables outside of the chainweb-data ORM causes an error.

I.e. add a new table, add a primary key. ORM checker throws an error.

I have fixed this and will open PR.

BA.TableConstraintRemoved{} -> False BA.ColumnConstraintRemoved{} -> False

enobayram commented 1 year ago

@mbwmbw1337 Thank you for opening this ticket as well as your #146 that fixes it. I agree that this issue is a genuine blocker for reasonable use cases, but I think #146 somewhat undermines the purpose of those checks. Ignoring the unexpected tables, views, enums etc. in the checkTables should be benign as far as our ORM is concerned, but ignoring new columns and constraints certainly might not be. For example, if you add a new column without a default value, then you would only get an error the next time we try to insert a value through the ORM. That said, there are of course many ways you could add new constraints and columns and they could still be compatible with the ORM, so we definitely don't want to stop the user from doing that as long as the user takes responsibility for it.

In order to support such use cases without reducing the safety of the schema checks, I've just opened #147 so that those checks are never fatal unless you go out of your way to ask for them to be. Let me know if this works for you.

mbwmbw1337 commented 1 year ago

@enobayram -- thank you for the quick and well-formulated response and your work on this project. I agree #147 is an even better and more elegant solution to addressing my concerns. So, please don't hesitate to reject my pull request, as #147 solves the issue! Thank you.