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

Add AdditionalQuery as function parameter for :one and :many queries #2636

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

RadhiFadlillah
Copy link
Contributor

@RadhiFadlillah RadhiFadlillah commented Aug 19, 2023

What is this?

This PR is implementation for my proposal in #2021 to allow additional query as function parameter in :one and :many queries. I've explained my reasoning there, but I will rewrite it again here for clarity.

Why?

One of the most common feature request for CRUD and BI apps is to make an advanced data filter where the users can freely define the parameters to specify which data to be shown. However, since sqlc act as compiler for SQL queries to Go code, once the compilation finished it can't be modified anymore. Thanks to this, advanced data filter is impossible to done properly with sqlc.

To solve this issue, this PR add AdditionalQuery as function parameter in the compiled :one and :many queries. This way we still able to create advanced data filter while retaining the advantage of compiling the SQL queries.

Example

For example, let's say we have queries like this:

-- Example queries for sqlc
CREATE TABLE IF NOT EXISTS product (
	id           INT UNSIGNED  NOT NULL AUTO_INCREMENT,
	identifier   VARBINARY(20) DEFAULT NULL,
	category     VARCHAR(80)   NOT NULL,
	name         VARCHAR(80)   NOT NULL,
	qty          DECIMAL(20,4) NOT NULL,
	capital      DECIMAL(20,4) NOT NULL,
	price        DECIMAL(20,4) NOT NULL,
	specs        JSON          DEFAULT NULL,
	PRIMARY KEY (id),
	UNIQUE KEY product_identifier_UNIQUE (identifier)
) CHARACTER SET utf8mb4;

-- name: FetchProducts :many
SELECT id, category, identifier, name, capital, price
FROM product;

From this query, sqlc will generate following Go code:

const FetchProducts = `-- name: FetchProducts :many
SELECT id, category, identifier, name, capital, price
FROM product;
`

type FetchProductsRow struct { // Omitted }

func (q *Queries) FetchProducts(ctx context.Context, db DBTX) ([]FetchProductsRow, error) {
	rows, err := db.QueryContext(ctx, FetchProducts)
	if err != nil {
		return nil, err
	}
	defer rows.Close()
	items := []FetchProductsRow{}
	// Omitted
	return items, nil
}

As you can see, currently our query for FetchProducts is run immediately by db.QueryContext, making it impossible to define custom SQL filter.

To allow custom SQL filter, this PR will add AdditionalQuery as function parameter for SELECT queries. Here is the generated Go code with this PR:

type AdditionalQuery struct {
   SQL  string
   Args []any
}

const FetchProducts = `-- name: FetchProducts :many
SELECT id, identifier, name, capital, price FROM product
`

type FetchProductsRow struct { // Omitted }

func (q *Queries) FetchProducts(ctx context.Context, db DBTX, aq ...AdditionalQuery) ([]FetchProductsRow, error) {
   query := FetchProducts
   queryParams := []interface{}{}

   if len(aq) > 0 {
   	query += " " + aq[0].SQL
   	queryParams = append(queryParams, aq[0].Args...)
   }

   rows, err := db.QueryContext(ctx, query, queryParams...)
   if err != nil {
   	return nil, err
   }
   defer rows.Close()
   items := []FetchProductsRow{}
   // Omitted
   return items, nil
}

With that new parameter, now we can define custom filter like this:

sql := `WHERE (
	category LIKE ?
	AND name LIKE ?
	AND capital < ?)`
args := []interface{}{
	"%Man%",
	"%Shirt%",
	500}
filter := AdditionalQuery{SQL: sql, Args: args}
products, err := FetchProducts(ctx, db, filter)

Difference with the proposal

If you've read my proposal before, you'll notice there is one difference between the proposal and this PR: in proposal I suggest to wrap the SELECT query inside sub-query.

I decided to not implement it because there might be performance issue coming from using sub-query. Not to mention from the user perspective (i.e. other developers who use sqlc), usually they don't like if the compiler modify their SQL query without any information.

Pros and cons of this PR

The pros are:

  • Making data filter is really easy now, since we can simply add whatever SQL queries that we want. Besides for filter, we can also use the additional SQL for other purpose, e.g. data sorting or limit.
  • For sqlc users, since the AdditionalQuery parameter is variadic, it means the generated code can be used as it is so there are no need to modify the existing code.

I'm not really sure about the cons. I've used this PR in production and it seems to be fine. However I'm only using it for basic SELECT queries, so there might be some edge cases that I'm not aware of.

Status of this PR

I've separated this PR into several commits:

  • 209c3df is the main change in this PR which modified the template files.
  • c28d048 contains files that generated by make regen.
  • e874732 fix test diff_output and diff_no_output.
  • f15bd18 fix test for process_plugin_disabled.

@RadhiFadlillah
Copy link
Contributor Author

By the way, while working on this PR I found three possible issues:

  1. make regen only generate test data for directory where command inside its exec.json is generate. Thanks to this, test data for diff_output and diff_no_output must be done manually.

  2. make regen ignores env property in exec.json. Thanks to this, stderr.txt in process_plugin_disabled will be cleared out by make regen, which lead to test failed.

  3. Currently sqlc doesn't reserve any keyword which used by Go since 1.18. Thanks to this, test in testdata/any currently failed. As I've mentioned before, I declared AdditionalQuery struct like this:

    type AdditionalQuery struct {
        SQL  string
        Args []any
    }

    However in testdata/any that any keyword already used as query name, which is why the test failed.

@Jille
Copy link
Contributor

Jille commented Aug 24, 2023

If we were to add extra, optional arguments to every query, we should follow the options pattern so that we could later also pass other things than extra pieces of query.

For example we'd introduce sqlc.Option which is an interface implementing an (unexported?) method. AdditionalQuery would implement that interface, and any future options we'd want could also conform to that interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants