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

Support OF in locking clause #285

Closed
mattdowdell opened this issue Oct 30, 2023 · 5 comments · Fixed by #288
Closed

Support OF in locking clause #285

mattdowdell opened this issue Oct 30, 2023 · 5 comments · Fixed by #288

Comments

@mattdowdell
Copy link
Contributor

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
Copy link
Owner

go-jet commented Oct 30, 2023

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

@go-jet go-jet modified the milestones: Version 2.12.0, Version 2.11.0 Oct 30, 2023
@mattdowdell
Copy link
Contributor Author

@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
Copy link
Owner

go-jet commented Oct 31, 2023

No, I don't see other workaround.

@mattdowdell
Copy link
Contributor Author

mattdowdell commented Nov 20, 2023

@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
Copy link
Owner

go-jet commented Nov 21, 2023

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.

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

Successfully merging a pull request may close this issue.

2 participants