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

Load() calculate hash of every table at the end and I don't know why #203

Open
slsyy opened this issue Jun 24, 2024 · 4 comments
Open

Load() calculate hash of every table at the end and I don't know why #203

slsyy opened this issue Jun 24, 2024 · 4 comments

Comments

@slsyy
Copy link

slsyy commented Jun 24, 2024

v3.11.0

The last function call for Load():

return l.helper.afterLoad(l.db)

and postgres implementation

testfixtures/postgresql.go

Lines 362 to 376 in b1fe75b

func (h *postgreSQL) afterLoad(q queryable) error {
if h.tablesChecksum != nil {
return nil
}
h.tablesChecksum = make(map[string]string, len(h.tables))
for _, t := range h.tables {
checksum, err := h.getChecksum(q, t)
if err != nil {
return err
}
h.tablesChecksum[t] = checksum
}
return nil
}

Basically it iterates over the whole database (in my case it is 100+ tables) and calculate md5 hash of each table for no reason. Do I miss something or maybe it can be removed? In my case it slows down tests in noticeable way

@slsyy
Copy link
Author

slsyy commented Jun 25, 2024

Oh I see, it make sense for subsequent Load calls.

Do you think it make sense to make some flag to disable this behavior? In my case I use Load only once per loader, because I use https://github.com/DATA-DOG/go-txdb , which perform cleanup automatically

@andreynering
Copy link
Contributor

Hi @slsyy,

We already have DangerousSkipCleanupFixtureTables. Can you try and see if it works for you?

I'm not sure if it also skips the checksum computing for all tables, but if not it would make sense to fix that IMO.

@slsyy
Copy link
Author

slsyy commented Jun 25, 2024

@andreynering this flag control that piece of code:

if !l.skipCleanup {
for _, file := range l.fixturesFiles {
modified := modifiedTables[file.fileNameWithoutExtension()]
if !modified {
continue
}
if err := file.delete(tx, l.helper); err != nil {
return err
}
}
}

where I want to skip this:

return l.helper.afterLoad(l.db)

Reason: In my code I use this library in such a way:

	fixtures, err := testfixtures.New(...)
	handleErr(t, err)
	handleErr(fixtures.Load());
	// and fixtures object dies here

so I basically don't need all of those hashes, because they won't be used anyway, if I don't call Load() more than once. With txdb anyway it doesn't make sense as tx.Rollback revert all the changes

I can raise a PR, if it make sense

@andreynering
Copy link
Contributor

@slsyy I'm not so sure if we could do the fix for the existing DangerousSkipCleanupFixtureTables flag, or if we'd need a new flag this. I'd need to revisit the code to be sure.

Anyway, feel free to open a PR with a proposal if you want to!

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

No branches or pull requests

2 participants