pggen icon indicating copy to clipboard operation
pggen copied to clipboard

proposal: Make generation of Batch and Scan optional

Open sadmansakib opened this issue 4 years ago • 4 comments
trafficstars

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;

sadmansakib avatar May 14 '21 13:05 sadmansakib

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.

jschaf avatar May 17 '21 08:05 jschaf

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.

sadmansakib avatar May 17 '21 11:05 sadmansakib

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.

jschaf avatar May 17 '21 19:05 jschaf

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

sadmansakib avatar May 17 '21 21:05 sadmansakib