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

Factories generate ID's outside the int32 range #158

Closed
Joshswooft opened this issue Jan 17, 2024 · 15 comments
Closed

Factories generate ID's outside the int32 range #158

Joshswooft opened this issue Jan 17, 2024 · 15 comments
Labels
bug Something isn't working

Comments

@Joshswooft
Copy link

Joshswooft commented Jan 17, 2024

I'm trying to seed my database with some random data using the factories bob generates from my schema and noticed that my script is failing due to the number faker generates is above the threshold for my ID field.

bob version: v0.23.2
jaswdr faker version: v1.19.1

Schema:

CREATE TABLE employee(
    id SERIAL PRIMARY KEY,
    first_name VARCHAR(60) NOT NULL,
);
// db setup excluded
bobDB := bob.New[*sql.DB](db)
dbFactory := factory.New()
seed := 42
source := rand.NewSource(seed)
myFaker := faker.NewWithSeed(source)
dbFactory.AddBaseEmployeeMod(factory.EmployeeMods.RandomizeAllColumns(&myFaker))
tmpl:= dbFactory.NewEmployee()

_, err = tmpl.CreateMany(ctx, bobDB, 10)

if err != nil {
	log.Fatal("failed to create employees: ", err)
}

Error I receive:

failed to encode args[0]: unable to encode omit.Val[int]{value:608747136543856411, state:1} into binary format for int4 (OID 23): unable to encode 608747136543856411 into binary format for int4 (OID 23): 608747136543856411 is greater than maximum value for int4

Under the hood the RandomizeAllColumns will call the following bob code:

func (m employeeMods) RandomID(f *faker.Faker) EmployeeMod {
	return EmployeeModFunc(func(o *EmployeeTemplate) {
		o.ID = func() int {
			return random[int](f)
		}
	})
}

Sure enough in my debugger If i inspect tmpl.ID() I get back 608747136543856411 which is larger than the max int32 (2,147,483,647)

which later causes the error in the bob library exec.goL114

	rawSlice, err := scan.All(ctx, exec, m, sql, args...) // fails here
	if err != nil {
		return nil, err
	}
@Joshswooft
Copy link
Author

Joshswooft commented Jan 17, 2024

I've managed to temporarily get around this issue by supplying my own mod that implements the EmployeeMod interface. So others can also benefit from this workaround I'll supply the solution below:

type EmployeeModFunc func(*factory.EmployeeTemplate)

func (f EmployeeFunc) Apply(n *factory.EmployeeTemplate) {
	f(n)
}

// Generates Random int32
func RandomID(f *faker.Faker) factory.EmployeeMod {
	return EmployeeModFunc(func(o *factory.EmployeeTemplate) {
		o.ID = func() int {
			return rand.Intn(1<<31 - 1)
		}
	})
}

The generation slightly changes to:

dbFactory.AddBaseEmployeeMod(factory.EmployeeMods.RandomizeAllColumns(&myFaker), RandomID(&myFaker))

Hope this helps anyone facing this issue until a solution is found :)

PS. I am loving this library so far, particularly the type-safe query building so please keep up the good work 💯

@stephenafamo
Copy link
Owner

stephenafamo commented Jan 17, 2024

So, the real issue is that the database driver should set the type of the column to int32 and not int

What codegen driver are you using?

Side note, to supply custom mods, you can write much less code by using one of the generated column mods.

dbFactory.AddBaseEmployeeMod(
    factory.EmployeeMods.RandomizeAllColumns(&myFaker),
    factory.EmployeeMods.IDFunc(func() int { return rand.Intn(1<<31 - 1) }),
)

Also, if you do not need the faker to be initialized with a specific seed, you can use nil and the default faker will be used.

@stephenafamo stephenafamo added the bug Something isn't working label Jan 17, 2024
@Joshswooft
Copy link
Author

Joshswooft commented Jan 17, 2024

So, the real issue is that the database driver should set the type of the column to int32 and not int

What codegen driver are you using?

Side note, to supply custom mods, you can write much less code by using one of the generated column mods.

dbFactory.AddBaseEmployeeMod(
    factory.EmployeeMods.RandomizeAllColumns(&myFaker),
    factory.EmployeeMods.IDFunc(func() int { return rand.Intn(1<<31 - 1) }),
)

Also, if you do not need the faker to be initialized with a specific seed, you can use nil and the default faker will be used.

I'm using the postgres driver. (Using postgres v15).

Thanks for the additional information I'll make sure to refactor my solution :)

Ps. Seed is needed for my scenario to ensure that the generated data is consistent each time

@stephenafamo
Copy link
Owner

I'm using the postgres driver. (Using postgres v15).

I'll also need to know the column type being mapped to int. Can you share the rough schema for the table?

@Joshswooft
Copy link
Author

Joshswooft commented Jan 18, 2024

Yeah this is the rough schema:

schema.sql

CREATE TABLE employee(
    id SERIAL PRIMARY KEY,
    first_name VARCHAR(60) NOT NULL,
    last_name VARCHAR(120) NOT NULL,
    full_name VARCHAR(255) GENERATED ALWAYS AS (first_name || ' ' || last_name) STORED
);

(Atlas version: atlas version v0.13.2-93d5aeb-canary)
I then use atlas to generate the desired table states using this command:

atlas schema apply -u ${DB_URL} --to="file://schema.sql" --env ${ATLAS_ENV}

To find the underlying table structure I run this query:

query:

SELECT column_name, data_type
FROM information_schema.columns
WHERE table_name = 'employees'
ORDER BY ordinal_position;

Which prints:

 column_name |     data_type     
-------------+-------------------
 id          | integer
 first_name  | character varying
 last_name   | character varying
 full_name   | character varying
(4 rows)

Seems to be correct according to the postgres docs: https://www.postgresql.org/docs/current/datatype-numeric.html#DATATYPE-SERIAL

@stephenafamo
Copy link
Owner

To clarify, are you using bobgen-psql or bobgen-atlas?

@Joshswooft
Copy link
Author

To clarify, are you using bobgen-psql or bobgen-atlas?

I'm using this command to generate the ORM:

PSQL_DSN=${DB_URL} go run github.com/stephenafamo/bob/gen/bobgen-psql@latest -c config/bobgen.yaml

Then my bobgen.yaml

psql:
  output: generated/models
  except:
    public.migrations:

I want to use the bobgen-atlas in the future but atm I can't seem to connect to my table using it but the boben-psql is working fine for now :)

@stephenafamo
Copy link
Owner

bobgen-atlas takes a dir configuration for where to find HCL files. But since you're using SQL files, directly connecting to the database may be cleaner.

@Joshswooft
Copy link
Author

Good to know 👍, I'm new to atlas and HCL thats why I'm keeping to SQL for now. Although I suppose I can always generate the hcl file from my schema.sql 🤔 if this is a problem that doesn't happen in the bobgen-atlas library.

@Joshswooft
Copy link
Author

Joshswooft commented Jan 18, 2024

When you map through the postgres serial types

	case *postgres.SerialType:
		switch t.T {
		case "smallserial", "serial2":
			c.Type = "int16"
		case "serial", "serial4":
			c.Type = "int"
		case "bigserial", "serial8":
			c.Type = "int64"
		default:
			c.Type = "int"
		}

Should you change the type to int32 if it's serial or serial4? Probably being naive here and haven't looked at what other impact this would cause

@stephenafamo
Copy link
Owner

This should already be fixed on the main branch.

A number of improvements have been made for randomization, including generated tests for randomization.
While running those generated tests, I did come across this and already fixed integrer to be generated as int32

Try running from the latest commit, and let me know if you're still running into issues.

@stephenafamo
Copy link
Owner

I've updated bobgen-atlas to fix this too in #163

@Joshswooft
Copy link
Author

Joshswooft commented Jan 18, 2024

@stephenafamo latest commit worked for me thanks 🦄 🦖 🔥, can we expect a new release to come out with this fix?

@stephenafamo
Copy link
Owner

can we expect a new release to come out with this fix?

Likely over the weekend. Can't think of anything else I want to add before a release

@stephenafamo
Copy link
Owner

Released v0.24.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants