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

proposal: Make generation of Batch and Scan optional #33

Open
sadmansakib opened this issue May 14, 2021 · 4 comments
Open

proposal: Make generation of Batch and Scan optional #33

sadmansakib opened this issue May 14, 2021 · 4 comments

Comments

@sadmansakib
Copy link

I think generation of Batch and Scan for every query is unnecessary. For most cases they are unused and untested codes since we don't need those in our business logic. I propose turning off Batch and Scan by default and enable them for certain queries using a flag on sql files. For example:

the following query with this flags should not generate Batch and Scan

    -- name: InsertAuthor :one
    INSERT INTO author (first_name, last_name)
    VALUES (pggen.arg('FirstName'), pggen.arg('LastName'))
    RETURNING author_id;

the following query with this flags should generate Batch and Scan

    -- name: InsertAuthor :one --include-batch
    INSERT INTO author (first_name, last_name)
    VALUES (pggen.arg('FirstName'), pggen.arg('LastName'))
    RETURNING author_id;
@jschaf
Copy link
Owner

jschaf commented May 17, 2021

Yea, in retrospect, I think the Batch and Scan variants should live in a separate BatchQuerier interface. That would at least solve large number of similarly named methods on Querier.

I'm trying really hard to avoid flags since each flag increases the number of combinations that need to be tested. That's the reason pggen always generates the batch methods. I'm okay with pushing some of the customization burden onto clients but maybe there's an easy middle ground. What if pggen generated all batch and scan queries into a defined section you could remove with sed?

// start batch queries
interface BatchQuerier {
  // Batch & scan methods
}

func (db *DBQuerier) QueryBatch() {}
// end batch queries

Then, after running pggen, remove batch queries with sed -i '/start batch queries/,/end batch queries/d' *.sql.go.

@sadmansakib
Copy link
Author

A separate interface for all batch functions is a cleaner approach. But will it generate a separate file for the interface or the interface will share the same file with original Querier interface. I think using a separate file will make it manageable if a user is supposed to run sed script.

@jschaf
Copy link
Owner

jschaf commented May 17, 2021

But will it generate a separate file for the interface or the interface will share the same file with original Querier interface.

Same file. I like 1 file per input file since it's easy to figure out where the code came from.

@sadmansakib
Copy link
Author

Also i would like to mention that as far as i can remember windows doesn't have a sed like utility. If separate interface is implemented, which requires client side sed script to cleanup unused functions then its better to add this on documentation as an optional clause

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