Adding LIMIT clause to query results in assertion failure

[tpolecat]

The following query

Select(
  "programs", 
  Nil, 
  Filter(
    And(
      Or(
        Contains(ListPath(List("users", "userId")), Const("u-107")), 
        Eql(UniquePath(List("piUserId")), Const("u-107"))
      ), 
      Eql(UniquePath(List("existence")), Const(Present))
    ), 
    Group(
      List(
        Select("id", List(), Empty),  
        Select("name", List(), Empty)
      )
    )
  )
)

produces the following SQL.

SELECT
  t_program.c_name,
  t_program.c_program_id
FROM
  t_program
  INNER JOIN (
    SELECT
      DISTINCT ON ((t_program.c_program_id COLLATE "C"))(t_program.c_program_id COLLATE "C")
    FROM
      t_program
      LEFT JOIN t_program_user ON (
        t_program_user.c_program_id = t_program.c_program_id
      )
    WHERE
      (
        (
          (
            (t_program_user.c_user_id = $1)
            OR (t_program.c_pi_user_id = $2)
          )
          AND (t_program.c_existence = $3)
        )
      )
      AND (t_program.c_program_id IS NOT NULL)
    ORDER BY
      (t_program.c_program_id COLLATE "C") NULLS LAST
  ) AS t_program_pred ON (
    t_program_pred.c_program_id = t_program.c_program_id
  )

Adding a Limit to the child query, thus

Select(
  "programs",
  Nil,
  Filter(
    And(
      Or(
        Contains(ListPath(List("users", "userId")), Const("u-107")),
        Eql(UniquePath(List("piUserId")), Const("u-107"))
      ),
      Eql(UniquePath(List("existence")), Const(Present))
    ),
    Limit(
      1000,
      Group(
        List(
          Select("id", Nil, Empty), 
          Select("name", Nil, Empty)
        )
      )
    )
  )
)

results in

java.lang.AssertionError: assertion failed
	at scala.Predef$.assert(Predef.scala:264)
	at edu.gemini.grackle.sql.SqlMapping$SqlQuery$SqlSelect.addFilterOrderByOffsetLimit(SqlMapping.scala:1717)
        ...

due to (I assume) the already-present ORDER BY caused by the joins ... ?

assert(orders.isEmpty && offset.isEmpty && limit.isEmpty && !isDistinct)

Relevant [abbreviated] definitions:

create table t_user (
  c_user_id     d_user_id     primary key not null,
  ...
);

create table t_program (
  c_program_id  d_program_id  not null primary key,
  c_existence   e_existence   not null,
  c_pi_user_id  d_user_id,
  c_name        text,
  ...
);

create table t_program_user (
  c_program_id  d_program_id  not null,
  c_user_id     d_user_id     not null,
  ...
);

object Program extends TableDef("t_program") {
  val Id        = col("c_program_id", program_id)
  val PiUserId  = col("c_pi_user_id", user_id)
  val Existence = col("c_existence", existence)
  val Name      = col("c_name", text_nonempty.opt)
}
object ProgramUser extends TableDef("t_program_user") {
  val ProgramId = col("c_program_id", program_id)
  val UserId    = col("c_user_id", user_id)
  val Role      = col("c_role", program_user_role)
}
object User extends TableDef("t_user") {
  val Id = col("c_user_id", user_id)
}

ObjectMapping(
  tpe = QueryType,
  fieldMappings = List(
    SqlRoot("programs"),
    SqlRoot("program"),
  )
),
ObjectMapping(
  tpe = ProgramType,
  fieldMappings = List(
    SqlField("id", Program.Id, key = true),
    SqlField("existence", Program.Existence, hidden = true),
    SqlField("name", Program.Name),
    SqlField("piUserId", Program.PiUserId, hidden = true),
    SqlObject("pi", Join(Program.PiUserId, User.Id)),
    SqlObject("users", Join(Program.Id, ProgramUser.ProgramId)),
  ),
),
ObjectMapping(
  tpe = ProgramUserType,
  fieldMappings = List(
    SqlField("programId", ProgramUser.ProgramId, hidden = true, key = true),
    SqlField("userId", ProgramUser.UserId, key = true),
    SqlField("role", ProgramUser.Role),
    SqlObject("user", Join(ProgramUser.UserId, User.Id))
  ),
),
LeafMapping[lucuma.core.model.User.Id](UserIdType),
LeafMapping[lucuma.core.model.Program.Id](ProgramIdType),
LeafMapping[ProgramUserRole](ProgramUserRoleType),

[milessabin]

Whilst there is a bug here, I think you almost certainly want to swap the order of the Filter and the Limit.

In general, given a child query yielding a list of values, you would first filter, then order, then offset, then limit, ie. (filter compose order compose offset compose limit)(child) or limit(offset(order(filter(child)))).

So what's happened is that the FilterOrderOffsetLimit extractor sees this as two separate operations, an inner one which is a Limit only and an outer one which is a Filter only. That shouldn't result in an assert, however, as I said above, it most likely isn't what you want here.

[tpolecat]

(Same result if I use the FilterOrderOffsetLimit constructor, which presumably nests them in the right order.)