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.

Sign up for GitHub

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

Jump to bottom

Respect character_maximum_length when generating random values for string columns #241

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Respect character_maximum_length when generating random values for string columns #241

wants to merge 1 commit into from
+58 −22

Conversation

mbezhanov
Copy link
Contributor

Hi, I'm opening this PR to propose a new feature that addresses an issue I ran into while trying out the ORM generators.

Consider the following PostgreSQL schema:

CREATE TABLE pilots (
    id serial PRIMARY KEY,
    name varchar(5) NOT NULL
);

CREATE TABLE jets (
  id serial PRIMARY KEY,
  pilot_id int NOT NULL,
  first_name varchar(7) NOT NULL,
  last_name varchar(8) NOT NULL,
  FOREIGN KEY (pilot_id) REFERENCES pilots(id)
);

CREATE INDEX idx_jets_pilots ON jets (pilot_id);
CREATE INDEX idx_jets_names ON jets (first_name, last_name);

Or, alternatively, the following MySQL schema:

CREATE TABLE pilots (
    id int unsigned NOT NULL,
    name varchar(5) NOT NULL,
    primary key (id)
);

CREATE TABLE jets (
  id int unsigned NOT NULL,
  pilot_id int unsigned NOT NULL,
  first_name varchar(7) NOT NULL,
  last_name varchar(8) NOT NULL,
  primary key (id)
);

CREATE INDEX idx_jets_pilots ON jets (pilot_id);
CREATE INDEX idx_jets_names ON jets (first_name, last_name);
ALTER TABLE jets ADD CONSTRAINT fkey_pilots_1 FOREIGN KEY(pilot_id) REFERENCES pilots(id);

Then try using the following code against either the MySQL or the PostgreSQL schema:

func main() {
	f := factory.New()
	f.AddBasePilotMod(
		factory.PilotMods.RandomID(nil),
		factory.PilotMods.RandomName(nil)
	)
	p := f.NewJet()

	ctx := context.Background()
	//ex, err := bob.Open("mysql", "test:test@/test?parseTime=true")
	ex, err := bob.Open("postgres", "postgres://postgres:test@localhost/postgres?sslmode=disable")
	if err != nil {
		log.Fatal(err)
	}
	_, err = p.CreateMany(ctx, ex, 5)
	if err != nil {
		log.Fatal(err)
	}
}

Bob fails with similar errors:

PostgreSQL:

2024/06/24 17:12:02 pq: value too long for type character varying(5)

MySQL:

2024/06/24 17:14:14 Error 1406 (22001): Data too long for column 'name' at row 1

That's because the random generator doesn't respect the specified character_maximum_length for the string columns. This PR changes this, and the random generators start producing strings truncated to the maximum possible length for each specified column. I hope that other Bob users will find this feature convenient too.

If you require any changes, just let me know 🙂

@mbezhanov mbezhanov force-pushed the respect-char-max-length branch from 84dc18a to 364eed0 Compare June 24, 2024 14:25
@mbezhanov
Copy link
Contributor Author

mbezhanov commented Jun 24, 2024 •
edited
Loading

I just realized this is possibly the same issue as the one reported in #178.

@mbezhanov mbezhanov force-pushed the respect-char-max-length branch from 364eed0 to 6f08035 Compare June 24, 2024 14:31
@stephenafamo
Copy link
Owner

Thanks for putting this together, however, I'm not sure if this is the best way to implement this.

Ideally, instead of having a dedicated truncateString function strictly for string types, I'd want to expand this to be able to possibly apply to other types.

I'll give this some more thought and share over the next week 🙏🏾

@mbezhanov
Copy link
Contributor Author

Sure, I'll be happy to rework this. Should I close this PR or leave it open for now to facilitate a discussion?

@stephenafamo
Copy link
Owner

Should I close this PR or leave it open for now to facilitate a discussion?

Let's leave it open for now

@stephenafamo stephenafamo linked an issue Nov 19, 2024 that may be closed by this pull request
Support for mods to respect database constraints #178
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for mods to respect database constraints
2 participants
@mbezhanov @stephenafamo