go-jet / jet

Type safe SQL builder with code generation and automatic query result data mapping
Apache License 2.0
2.52k stars 118 forks source link

Support `OF` in locking clause #285

Closed mattdowdell closed 10 months ago

mattdowdell commented 11 months ago

Is your feature request related to a problem? Please describe. I have a query doing LEFT JOIN across 2 tables modeled on a many-to-many relationship and locking rows to prevent concurrent updates.

My schema is as follows, minus some rows I trimmed out for simplification:

CREATE TABLE resources (
    id UUID PRIMARY KEY,
    name TEXT NOT NULL,
);

CREATE TABLE groups (
    id TEXT PRIMARY KEY,
    name TEXT NOT NULL
);

CREATE TABLE resources_groups (
    fk_resources_id UUID REFERENCES resources ON DELETE RESTRICT,
    fk_groups_id TEXT REFERENCES groups ON DELETE RESTRICT,
    PRIMARY KEY (fk_resources_id, fk_groups_id)
);

In essence I can have multiple resources and each resource can have multiple groups associated with it.

I then have an update API that executes a SQL query as follows:

SELECT
  resources.id AS "resources.id",
  resources.name AS "resources.name",
  groups.id AS "groups.id",
  groups.name AS "groups.name"
FROM public.resources
LEFT JOIN public.resources_groups ON (resources.id = resources_groups.fk_resources_id)
LEFT JOIN public.groups ON (groups.id = resources_groups.fk_groups_id)
WHERE (resources.id = 'd38f7d10-d265-4c10-b0c1-464d3af19d2b')
FOR UPDATE NOWAIT;

This produces an error:

ERROR: FOR UPDATE cannot be applied to the nullable side of an outer join (SQLSTATE 0A000)

This in itself seems to be expected per https://www.postgresql.org/message-id/21634.1160151923@sss.pgh.pa.us. And it's likely I need to rethink my use of joins in general.

On the plus side, I found that adjusting the locking clause to:

FOR UPDATE OF resources NOWAIT;

Allows the query to work. However, this does not seem to be supported in jet, potentially becaise this seems to be a PostgeSQL specific feature.

var m struct {
  Resource model.Resources
  Groups   []model.Groups
}

err := table.Resources.
  SELECT(table.Resources.AllColumns, table.Groups.AllColumns).
  FROM(
    table.Resources.
      LEFT_JOIN(table.ResourcesGroups, table.Resources.ID.EQ(table.ResourcesGroups.FkResourcesID)).
      LEFT_JOIN(table.Groups, table.Groups.ID.EQ(table.ResourcesGroups.FkGroupsID)),
  ).
  WHERE(table.Resources.ID.EQ(postgres.UUID(resourceID))).
  FOR(postgres.UPDATE().NOWAIT()). // No method named `OF` is available here
  QueryContext(ctx, conn, &m)

Describe the solution you'd like Support the use of OF in postgres.RowLock to allow outer joins to use locks.

In testing the revised query, I found that the table in the OF clause had to be just resources and the use of public.resources as used in the FROM clause did not work.

go-jet commented 11 months ago

Hi @mattdowdell, OF is not supported currently, but it makes sense to add it.

mattdowdell commented 11 months ago

@go-jet Is there a workaround I can use to inject OF <table name> into the locking clause? The best I thought of was emitting the SQL, doing a find/replace and then creating a raw statement to be executed by jet. But that seemed particularly clunky. I couldn't see any obvious hook to arbitrarily mutate the query before execution.

go-jet commented 11 months ago

No, I don't see other workaround.

mattdowdell commented 10 months ago

@go-jet I had a stab at implementing this, but got myself mixed up trying to figure out how. As it stands, RowLock's are part of the internal jet package, so extending the implementation there seems to be simplest. Particularly as that's where serialisation occurs today. I also added an extra interface/constructor (RowLockOf) so the method did not leak to the mysql or sqlite packages, e.g.

package jet

type RowLockOf interface {
  RowLock
  OF(table ...X) RowLock
}

func NewRowLockOf(name string) func() RowLockOf {
  // ...
}

The problem came when figuring out how to pass the tables into the OF clause. I assume I want postgres.ReadableTable as an argument, but that would cause a circular import. Would you be able provide some advice on how to tie it off and/or whether it's a viable implementation?

go-jet commented 10 months ago

As it stands, RowLock's are part of the internal jet package, so extending the implementation there seems to be simplest. Particularly as that's where serialisation occurs today.

Agree.

I also added an extra interface/constructor (RowLockOf) so the method did not leak to the mysql or sqlite packages, e.g.

There is no need for new interface. We want OF method to be accessible from mysql package, because mysql also supports naming a locking table.

The problem came when figuring out how to pass the tables into the OF clause. I assume I want postgres.ReadableTable as an argument, but that would cause a circular import. Would you be able provide some advice on how to tie it off and/or whether it's a viable implementation?

You don't need the whole postgres.ReadableTable interface, just TableName() and Alias() method for serialization purposes. You can use jet.Table interface as a parameter.