1

I have a function that abstracts updating event data:

async (
  connection: DatabaseConnectionType,
  id: number,
  venueId: number,
  movieId: number,
  cinemaMovieNameId: number,
  date: string,
  time: string,
  url: string | null,
  seatingIsAllocated: boolean | null,
  reservationFee: number | null,
  reservationPerTicketFee: number | null,
  guide: $PropertyType<EventResultType, 'guide'> | null
): Promise<number> => {
  const timezone = await connection.maybeOneFirst(sql`
    SELECT
      locality.timezone
    FROM venue
    INNER JOIN locality
    ON locality.id = venue.locality_id
    WHERE venue.id = ${venueId}
  `);

  if (!timezone) {
    log.error({
      venueId
    }, 'timezone not found');

    throw new Error('Timezone not found.');
  }

  // @todo Prevent unncessary updates. See updateVenue and updateTicket.
  await connection
    .query(sql`
      UPDATE
        event
      SET
        venue_id = ${venueId},
        movie_id = ${movieId},
        cinema_movie_name_id = ${cinemaMovieNameId},
        starts_at = ${date + ' ' + time}::timestamp AT TIME ZONE (${timezone}),
        url = ${url},
        seating_is_allocated = ${seatingIsAllocated},
        reservation_fee = ${reservationFee},
        reservation_per_ticket_fee = ${reservationPerTicketFee},
        guide = ${guide ? JSON.stringify(guide) : null}
      WHERE
        id = ${id}
    `);

  return id;
};

It seems redundant separate locality.timezone lookup query from the UPDATE query. However, combining these two queries requires asserting within a query that timezone exists.

Is there a syntax to assert how many results a query returns?

Something that would allow me to rewrite the above into ~:

await connection
  .query(sql`
    WITH timezone AS (
      SELECT
        locality.timezone
      FROM venue
      INNER JOIN locality
      ON locality.id = venue.locality_id
      WHERE venue.id = ${venueId}
      -- Made up syntax
      ASSERT ROW COUNT 1
    )
    UPDATE
      event
    SET
      venue_id = ${venueId},
      movie_id = ${movieId},
      cinema_movie_name_id = ${cinemaMovieNameId},
      starts_at = ${date + ' ' + time}::timestamp AT TIME ZONE ((SELECT timezone FROM timezone)),
      url = ${url},
      seating_is_allocated = ${seatingIsAllocated},
      reservation_fee = ${reservationFee},
      reservation_per_ticket_fee = ${reservationPerTicketFee},
      guide = ${guide ? JSON.stringify(guide) : null}
    WHERE
      id = ${id}
  `);
1
  • That is not valid SQL or PL/pgSQL - which programming language is that? As this seems related to the programming language (or Framework) rather than to Postgres itself, this should be asked on stackoverflow instead
    – user1822
    Commented May 7, 2018 at 6:36

3 Answers 3

1

It is not an assert, but you can just have the query return the empty string (rather than NULL) when there is no match:

... AT TIME ZONE ((SELECT timezone FROM timezone union select '' limit 1))

when invoked with empty timezone query gives:

ERROR:  time zone "" not recognized
1

You can use a case and a cast in a having clause to force an exception.

WITH timezone AS (
  SELECT
    max(locality.timezone) AS timezone
  FROM venue
  INNER JOIN locality
  ON locality.id = venue.locality_id
  WHERE venue.id = ${venueId}
  HAVING CASE count(distinct(locality.timezone))
         WHEN 1 THEN 'true'
         WHEN 0 THEN 'no timezone found'            
         ELSE 'too many timezones' END::boolean
)

The errors this will raise are like

invalid input syntax for type boolean: "no timezone found"

which is less than ideal, but at-least contains a dignotic message.

if you want better error messages than that you'll need to write an assert function that can be called from the case, instead of relying on the cast failing.

-1
    starts_at = ${date + ' ' + time}::timestamp AT TIME ZONE (${timezone}),

Oh my, that's not safe nor optimal. That's a really bad idea. First you don't know that date + ' ' + time isn't a SQL injection attack. You're simply not safely quoting those queries.

Not to mention the AT TIME ZONE is super sketch

starts_at = ${date + ' ' + time}::timestamp AT TIME ZONE (${timezone}),

This is setting starts_at with a timestamp without time zone as that's what AT TIME ZONE is returning. This is not what you want!

What you want is for all of your timestamps to be stored as timestamp with time zone. Your library should set the client_timestamp to UTC and pull them out in that format returning a JavaScript Date object. That can be used as is, or you can use a Moment.js to manipulate it from the client.

tldr: retrieve and set time stamps with UTC when possible. Don't rely on PostgreSQL to convert things to an arbitrary ${timezone} because the logic will be ugly. Instead use Moment.js.

2
  • This is making a lot of false assumptions about the code base. This code is neither vulnerable to SQL injections, nor is the use of AT TIME ZONE unintentional. Does not answer the original question.
    – Gajus
    Commented May 6, 2018 at 18:25
  • If you would like to learn about how variable interpolation mechanism works, refer to github.com/gajus/slonik. Explaining the intent behind the use of AT TIME ZONE is out of scope. Your content would better suit as a comment, rather than an answer.
    – Gajus
    Commented May 6, 2018 at 18:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.