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

fix: fix the bug of the numeric/decimal data type in pg #4686

Merged

Conversation

POABOB
Copy link
Contributor

@POABOB POABOB commented Mar 5, 2025

Issue #4624.

I tried the PostgreSQL DDL and use both numeric and decimal type to generate the model.
balance_test_tab.sql

CREATE TABLE balance_test_tab (
      id bigserial NOT NULL,
      balance numeric(78) DEFAULT 0 NOT NULL,
      CONSTRAINT balance_test_tab_pk PRIMARY KEY (id)
);

CREATE TABLE balance_test_tab (
      id bigserial NOT NULL,
      balance decimal(78) DEFAULT 0 NOT NULL,
      CONSTRAINT balance_test_tab_pk PRIMARY KEY (id)
);

Output

These cases both gave the int64 type to the code.

BalanceTestTab struct {
		Id      int64 `db:"id"`
		Balance int64 `db:"balance"`
}

Otherwise, I use DDL in MySQL below:

CREATE TABLE balance_test_tab (
  id BIGINT AUTO_INCREMENT NOT NULL,
  balance DECIMAL(78,0) DEFAULT 0 NOT NULL,
  PRIMARY KEY (id)
);

Output

The type was float64.

	BalanceTestTab struct {
		Id      int64   `db:"id"`
		Balance float64 `db:"balance"`
	}

Key Reason

When goctl use datasource to get the columns info, it will transfer the type to mysql type. The numeric/decimal was transfer to the bigint type, and it was not reasonable.

tools/goctl/model/sql/model/postgresqlmodel.go

var p2m = map[string]string{
	"int8":        "bigint",
	"numeric":     "double", // it was bigint, after I changed it 
	"decimal":     "double", // add it to support the type
	"float8":      "double",
	"float4":      "float",
	"int2":        "smallint",
	"int4":        "integer",
	"timestamptz": "timestamp",
}

func (m *PostgreSqlModel) convertPostgreSqlTypeIntoMysqlType(in string) string {
	r, ok := p2m[strings.ToLower(in)]
	if ok {
		return r
	}

	return in
}

Copy link

codecov bot commented Mar 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.55%. Comparing base (8690859) to head (b61eae7).
Report is 284 commits behind head on master.

Additional details and impacted files

see 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kevwan kevwan requested a review from kesonan March 5, 2025 10:52
Copy link
Collaborator

@kesonan kesonan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kevwan kevwan added this pull request to the merge queue Mar 7, 2025
Merged via the queue into zeromicro:master with commit 967f092 Mar 7, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants