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

RDBMS schema management #490

Closed
darh opened this issue Nov 11, 2022 · 8 comments
Closed

RDBMS schema management #490

darh opened this issue Nov 11, 2022 · 8 comments
Assignees
Labels
backend Backend server changes (GO) enhancement New feature or request
Milestone

Comments

@darh
Copy link
Contributor

darh commented Nov 11, 2022

Allow integrators to manipulate RDBMS schema (make, change and remove database tables) when fields are added, changed or removed on a Compose module.

DAL package (pkg/dal) can already produce a diff between the two models.

Implementation (store/adapters/rdbms/dal/connection.go) already has placeholders:

  • Models
  • CreateModel
  • DeleteModel
  • UpdateModel
  • UpdateModelAttribute
@darh darh added this to the 2023.3.0-dev.4 milestone Nov 11, 2022
@darh darh added enhancement New feature or request backend Backend server changes (GO) labels Nov 11, 2022
@github-actions
Copy link

Stale issue message

@tjerman
Copy link
Member

tjerman commented Feb 20, 2023

@skamensky IIRC you and Denis were chatting about you potentially handling this one. Is this still the case/what's the progress on it?

@skamensky
Copy link
Contributor

skamensky commented Feb 20, 2023

@tjerman

Yes, I was working on this. As of now these things are either handled or have workarounds.

I think you added most of the implementation mentioned in the description of the issue.

  • Models - can be added or removed - but not updated
  • CreateModel - done (by you?)
  • DeleteModel - done (by you?)
  • UpdateModel - this is not supported. I'm not sure what it would look like anyway. Changing the underlying dal handle?
  • UpdateModelAttribute - This is also unsupported and - for my personal needs - the most missed. Currently I must either delete a column and re-add it (which means a data migration), or modify things manually on the compose_module_field level and against the DB directly. I think as a start, there could be a list of non destructive changes that should be supported (e.g. changing a type from int to bigint) and an error thrown when the conversion cannot be done safely.

My changes can be found in this commit:
9ac6387

I'm currently putting all contributions on the side since I'm in the weeds implementing Corteza instead of extending it.
I expect to have more contribution time available in May, but when I jump back in, I plan to focus on RBAC bulk read optimizations instead of this issue.

@Fajfa Fajfa modified the milestones: 2023.3.0-dev.4, 2023.3.0-dev.5 Mar 7, 2023
@katrinDY
Copy link
Contributor

Tomaž to revisit this and make plan on what needs to be done

@tjerman
Copy link
Member

tjerman commented Mar 21, 2023

I'll hop onto it today

@tjerman
Copy link
Member

tjerman commented Mar 21, 2023

Issue #932 makes the current version a bit more robust; tasks outlined here will build on it to provide solid model manipulation.

For now, this will be a wall of text here. When we get to it, we'll split it into separate issues.

Todo

Allow model attribute changes

Currently, when a model is registered in DAL, the attribute can't change.
The reason behind this strict decision is that there will be an underlying database structure which may or may not support the change.

Adding and removing attributes is already implemented, but changing the codec and type is not.

The core logic for interacting with the database is stored in store/adapters/rdbms/dal/connection.go@UpdateModelAttribute

The core DAL logic for handling and managing models is stored in pkg/dal/service.go

These modifications require work both on the database layer (to do storage def. modifications) and on the DAL side (to update registries).

Allow model attribute codec changes

When the codec changes, we need to first update all of the values for that attribute.
The values can be changed using the provided dal.TransformationFunction function(s).

Note: a lot of this processing can be done concurrently.

  • Done

Allow model attribute type changes

When the type changes, we have these options:

  • If there is no data inside the table/module, we can simply change the type
  • if this change can be handled by the driver/database, we don't need to do much
  • define a list of conversions that can be offloaded to the driver/DB for MySQL
  • define a list of conversions that can be offloaded to the driver/DB for PgSQL
  • define a list of conversions that can be offloaded to the driver/DB for SQLite
  • define a list of conversions that can be offloaded to the driver/DB for SQLServer
  • if the new type is a superset of the old type (int -> bigint), the cast can happen automagically
  • define a list of "safe conversions" for MySQL
  • define a list of "safe conversions" for PgSQL
  • define a list of "safe conversions" for SQLite
  • define a list of "safe conversions" for SQLServer
  • if the new type is none of the above, we need to use the dal.TransformationFunction function(s)
  • if none of the above holds, we error out.
  • Done

Verify the flows

A lot of checks for verifying compatibility are done in the DAL service layer as well as in the DB layer.
These need to be revisited and updated to allow the new optional DB modifications (DB modifications are only done if explicitly enabled).

  • Done

Meta bits

  • pkg/dal/diff.go consider removing ModelModification and expand modelDiffType. I would prefer we update the original attribute to explicitly/implicitly denote what the change was.
  • store/adapters/rdbms/dal/connection.go@UpdateModelAttribute -- fmt.Errorf("cannot modify %s. Changing physical schemas is not yet supported", sampleAttribute.Ident) change the error to reflect what it actually is and for it to follow the guidelines.

@Fajfa Fajfa modified the milestones: 2023.3.0-dev.6, 2023.3.0-rc.1 Apr 7, 2023
@katrinDY katrinDY removed this from the 2023.3.0-rc.1 milestone Apr 13, 2023
@github-actions
Copy link

github-actions bot commented Jul 8, 2023

Stale issue message

@tjerman
Copy link
Member

tjerman commented Oct 16, 2023

Lost gem from a while back; to be reviewed and potentially incorporate bits and pieces into the already existing DAL schema alterations

@tjerman tjerman added this to the 2023.9.0 milestone Oct 16, 2023
@tjerman tjerman closed this as completed Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Backend server changes (GO) enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants