Dynamic ORDER BY and ASC / DESC in a plpgsql function

Question

Following the approach mentioned in this link, I want to pass ORDER BY and sorting order to a function dynamically.

ORDER BY is working fine but I am not able to pass sorting order (ASC / DESC).

What I have now:

CREATE OR REPLACE FUNCTION list(_limit integer,_offset integer,sort_by varchar(100), _order varchar(100),_category varchar(100))
  RETURNS TABLE(
     id INTEGER,
     name VARCHAR,
     clientname VARCHAR,
     totalcount BIGINT
  ) AS
$$
DECLARE
   empty text := '';
BEGIN
RETURN Query EXECUTE
'SELECT d.id,
d.name,
d.clientname,
 count(*) OVER() AS full_count FROM design_list as d 
    where ($5 = $6 Or d.category Ilike $5) 
        ORDER BY ' || quote_ident(sort_by) || ' LIMIT $1 offset $2'
USING _limit,_offset,sort_by, _order,_category, empty;
END;
$$  LANGUAGE plpgsql;

I don't get this at all. Why are you mixing concatenation with USING/$ in the same statement? Why are you putting 5 parameters in USING when you don't use $3 and $4 (sort_by and _order, as it happens)? You ORDER BY sort_by, don't use _order at all, and say ordering is working and sorting isn't??? — 404
– 404, Commented Apr 11, 2018 at 12:42

Erwin Brandstetter · Accepted Answer · 2021-03-14 17:19:25Z

6

I would do it like this:

CREATE OR REPLACE FUNCTION list(
      _category varchar(100)
    , _limit int
    , _offset int
    , _order_by varchar(100)
    , _order_asc_desc text = 'ASC')  -- last param with default value
  RETURNS TABLE(id int, name varchar, clientname varchar, totalcount bigint)
  LANGUAGE plpgsql AS
$func$
DECLARE
   _empty text := '';
BEGIN
   -- Assert valid _order_asc_desc
   IF upper(_order_asc_desc) IN ('ASC', 'DESC', 'ASCENDING', 'DESCENDING') THEN
      -- proceed
   ELSE
      RAISE EXCEPTION 'Unexpected value for parameter _order_asc_desc.
                       Allowed: ASC, DESC, ASCENDING, DESCENDING. Default: ASC';
   END IF;
   
   RETURN QUERY EXECUTE format(
     'SELECT id, name, clientname, count(*) OVER() AS full_count
      FROM   design_list
      WHERE ($1 = $2 OR category ILIKE $1) 
      ORDER  BY %I %s
      LIMIT  %s
      OFFSET %s'
    , _order_by, _order_asc_desc, _limit, _offset)
   USING _category, _empty;
END
$func$;

Core feature: use format() to safely and elegantly concatenate your query string. Related:

ASC / DESC (or ASCENDING / DESCENDING) are fixed key words. I added a manual check (IF ...) and later concatenate with a simple %s. That's one way to assert legal input. For convenience, I added an error message for unexpected input and a parameter default, so the function defaults to ASC if the last parameter is omitted in the call. Related:

Addressing Pavel's valid comment, I concatenate _limit and _offset directly, so the query is already planned with those parameters.

_limit and _offset are integer parameters, so we can use plain %s without the danger of SQL injection. You might want to assert reasonable values (exclude negative values and values too high) before concatenating ...

Other notes:

Use a consistent naming convention. I prefixed all parameters and variables with an underscore _, not just some.
Not using table qualification inside EXECUTE, since there is only a single table involved and the EXECUTE has its separate scope.
I renamed some parameters to clarify. _order_by instead of _sort_by; _order_asc_desc instead of _order.

edited Mar 14, 2021 at 17:19

answered Apr 11, 2018 at 12:54

Erwin Brandstetter

669k160 gold badges1.2k silver badges1.3k bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Pavel Stehule Over a year ago

LIMIT, OFFSET are important clauses for optimization - I am not sure if passing these parameters via USING clause can have negative impact on execution plan quality. This is classic example of single query wrapping - important and well known performance antipattern.

Erwin Brandstetter Over a year ago

@PavelStehule: Good point. I added an alternative for that as well.

Mike Packer · Accepted Answer · 2019-05-30 17:55:03Z

0

non dynamic sql solution.

CREATE OR REPLACE FUNCTION list(
...
in_use_asc boolean default false,
_order_by varchar(100)
..
)
..


CREATE TEMPORARY TABLE tempHolder ON COMMIT DROP AS
SELECT SELECT id, name, clientname, count(*) OVER() AS full_count
      FROM   design_list
      WHERE ($1 = $2 OR category ILIKE $1);

     IF in_use_asc = TRUE THEN
        RETURN QUERY SELECT * FROM tempHolder ORDER BY _order_by asc LIMIT {} OFFSET {};
    ELSE 
        RETURN QUERY SELECT * FROM tempHolder ORDER BY _order_by desc LIMIT {} OFFSET {};
    END IF;

Shouldnt be any slower because SQL needs to grab everything anyway because of the ORDER BY plus you avoid the dynamic SQL.

edited May 30, 2019 at 17:55

answered May 30, 2019 at 17:44

Mike Packer

835 bronze badges

1 Comment

ravioli Over a year ago

I don't think this will work. In your RETURN QUERY, it will just apply the ORDER BY to the literal string value passed in to _order_by_, which is a constant value and will thus negate any ordering. I only mention this because I'm trying to figure out the same problem and tried the same thing you did!

Collectives™ on Stack Overflow

Dynamic ORDER BY and ASC / DESC in a plpgsql function

2 Answers 2

2 Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related