gofr-dev / gofr

An opinionated GoLang framework for accelerated microservice development. Built in support for databases and observability.
https://gofr.dev
Apache License 2.0
1.59k stars 147 forks source link

[WIP] Support for AutoMigrate SQL #723

Closed reski-rukmantiyo closed 2 hours ago

reski-rukmantiyo commented 2 months ago

To complete about SQL CRUD journey, i think it's better to create similiar like AutoMigrate in Gorm. The reason is that to make consistent values between golang struct and sql table without manual interference.

The Automigrate function that I propose

Example golang struct

type Order struct {
    ID          int       `json:"id" gofr:"primary_key,auto_increment"`
    CustomerID  int       `json:"customer_id" gofr:"not_null,index,fk(Customers:ID)"`
    OrderDate   time.Time `json:"order_date" gofr:"not_null"`
    OrderAmount float64   `json:"order_amount" gofr:"not_null,check(order_amount > 0)"`
    Notes       string    `json:"notes" gofr:"size(1000),comment(This is a note field)"`
}

Output of create table SQL statement

CREATE TABLE Order (
    ID INT PRIMARY KEY AUTO_INCREMENT,
    CustomerID INTEGER NOT NULL,
    OrderDate DATETIME NOT NULL,
    OrderAmount REAL NOT NULL CHECK (order_amount > 0),
    Notes VARCHAR(1000) COMMENT 'This is a note field',
    FOREIGN KEY (CustomerID) REFERENCES Customers(ID)
);

CREATE INDEX idx_Order_CustomerID ON Order (CustomerID);

Code: #724

Please comments for these features

aryanmehrotra commented 2 months ago

Support for Drop table options if options True can you describe the use case for this?

reski-rukmantiyo commented 2 months ago

Support for Drop table options if options True can you describe the use case for this?

Sometimes we need to override existing table if necessary especially when we're in development stage. Just need to pass true in dropIfExists parameter

func GenerateCreateTableSQL(structType interface{}, dbType string, dropIfExists bool) (string, error) {
}
reski-rukmantiyo commented 2 months ago

Update by today,

Example golang struct

type Charge struct {
    ID             uint64    `json:"id" gofr:"primary_key,auto_increment"`
    OrganizationID int       `json:"organizationid" gofr:"fk(Organization:ID),not_null"`
    Period         time.Time `json:"period" gofr:"not_null"`
    Count          uint64    `json:"count" gofr:"unique,not_null,check(Count >= 0)"`
    ModelID        int       `json:"modelid" gofr:"fk(Model:ID),not_null,comment(this is a model ID for charge table)"`
    Comment        string    `json:"comment" gofr:"size(128),unique"`
}

will be

DROP TABLE IF EXISTS charge;

CREATE TABLE charge (
    id INT PRIMARY KEY AUTO_INCREMENT,
    organization_id INTEGER NOT NULL,
    period DATETIME NOT NULL,
    count INTEGER UNIQUE NOT NULL,
    model_id INTEGER NOT NULL COMMENT 'this is a model ID for charge table',
    comment VARCHAR(128) UNIQUE,
    FOREIGN KEY (organization_id) REFERENCES organization(id),
    FOREIGN KEY (model_id) REFERENCES model(id)
);

CREATE TRIGGER check_count_before_update
BEFORE UPDATE ON charge
FOR EACH ROW
BEGIN
    IF Count >= 0 THEN
        SIGNAL SQLSTATE '45000' SET MESSAGE_TEXT = 'Count >= 0';
    END IF;
END;
aryanmehrotra commented 2 months ago

Support for Drop table options if options True can you describe the use case for this?

Sometimes we need to override existing table if necessary especially when we're in development stage. Just need to pass true in dropIfExists parameter

func GenerateCreateTableSQL(structType interface{}, dbType string, dropIfExists bool) (string, error) {
}

How will the user use this?

reski-rukmantiyo commented 2 months ago

Support for Drop table options if options True can you describe the use case for this?

Sometimes we need to override existing table if necessary especially when we're in development stage. Just need to pass true in dropIfExists parameter

func GenerateCreateTableSQL(structType interface{}, dbType string, dropIfExists bool) (string, error) {
}

How will the user use this?

I revise the function into

func (a *App) AutoMigrate(dropIfExists bool, structs ...interface{}) error {
}

therefore user can access dropIfExists parameter to access the feature

Umang01-hash commented 2 months ago

@reski-rukmantiyo after some discussion with the team we have came to a point that at a particular time we don't want to deep dive into all the supported functionalities for SQL CRUD feature but we should first completely implement some basic necessary features and then we will keep updating and upgrading it with next advanced features.

For now please refer to below points :

Please work on adding the necessary things first and after that you can raise separate PR's for other advanced features.

reski-rukmantiyo commented 1 month ago

@reski-rukmantiyo after some discussion with the team we have came to a point that at a particular time we don't want to deep dive into all the supported functionalities for SQL CRUD feature but we should first completely implement some basic necessary features and then we will keep updating and upgrading it with next advanced features.

For now please refer to below points :

  • We currently don't need Support for drop table options if options true as the user can change the db schema or drop the table when in development process without much hassle. We will see if we will require it for advanced features.
  • Also the need for Suport to convert multiple structs at one time is not required as user can call the method CRUDFromStruct multiple time with different structs.
  • support for primary_key, auto_increment, not_null, unique, index, and unique_index,check,comment in gofr tags yes we will need the support for these sql constraints in our tags but we prefer sql instead of gofr as tag name.
  • support for int, float, bool, string and time.Time in JSON type this is required. (keep this in a different PR)
  • support foreign key as well this is also a valid requirement.

Please work on adding the necessary things first and after that you can raise separate PR's for other advanced features.

Noted @Umang01-hash , by the end of this week i will separate

While keep

Please confirm

Umang01-hash commented 1 month ago

Sure @reski-rukmantiyo moreover i will suggest let's do the second one first:

and merge it and then you can pick up the first

reski-rukmantiyo commented 1 month ago

Sure @reski-rukmantiyo moreover i will suggest let's do the second one first:

  • support for primary_key, auto_increment, not_null, unique, index, and unique_index,check,comment in gofr tags : change into sql meta tag
  • support for int, float, bool, string and time.Time in JSON type
  • support foreign key as well

and merge it and then you can pick up the first

Hi @Umang01-hash

Please check #800 . Thanks

ccoVeille commented 1 month ago

Currently reimplemented in

Umang01-hash commented 2 hours ago

This issue has been resolved in PR #930.