Thread

Dan Langille <dan@langille.org> — 2003-09-28T19:24:59Z

I'm trying to create a function which returns a result set using a dynamic
query.  The problem occurs when it compiles.  I suspect it's my quoting,
but I'm not sure of the cause.

CREATE OR REPLACE FUNCTION LoginCounts(int) RETURNS SETOF
logincounts_record AS '
DECLARE
    MaxDays ALIAS for $1;

    r   logincounts_record%rowtype;
    i   integer;

BEGIN
    FOR i IN 1..MaxDays LOOP
        EXECUTE ''
        SELECT count(*)
          INTO r
          FROM users
         WHERE lastlogin between current_date - interval \''' ||
quote_literal(i - 1) || '' days\'
                             AND current_date - interval \''' ||
quote_literal(i)     || '' days\''';

        RETURN NEXT r;
    END LOOP;
    RETURN;
END
'
LANGUAGE plpgsql;


# select * from LoginCounts(2);
WARNING:  Error occurred while executing PL/pgSQL function logincounts
WARNING:  line 9 at execute statement
ERROR:  parser: parse error at or near "days" at character 151

thnks

-- 
Dan Langille - http://www.langille.org/

Re:

Tom Lane <tgl@sss.pgh.pa.us> — 2003-09-28T19:45:22Z

Dan Langille <dan@langille.org> writes:
>          WHERE lastlogin between current_date - interval \''' ||
> quote_literal(i - 1) || '' days\'
>                              AND current_date - interval \''' ||
> quote_literal(i)     || '' days\''';

IIRC, quote_literal() puts single quotes around its result.  So you have
too many quotes there.  Given that you know i is an integer, you don't
really need quote_literal for it.  Actually, you don't need EXECUTE
here at all.  Why not just

    FOR i IN 1..MaxDays LOOP
        SELECT count(*)
          INTO r
          FROM users
         WHERE lastlogin between current_date - (i-1) * interval ''1 day''
                             AND current_date - i * interval ''1 day'';
        RETURN NEXT r;
    END LOOP;


			regards, tom lane

Re:

Dan Langille <dan@langille.org> — 2003-09-28T20:06:20Z

On 28 Sep 2003 at 15:45, Tom Lane wrote:

> Dan Langille <dan@langille.org> writes:
> >          WHERE lastlogin between current_date - interval \''' ||
> > quote_literal(i - 1) || '' days\'
> >                              AND current_date - interval \''' ||
> > quote_literal(i)     || '' days\''';
> 
> IIRC, quote_literal() puts single quotes around its result.  So you have
> too many quotes there.  Given that you know i is an integer, you don't
> really need quote_literal for it.  Actually, you don't need EXECUTE
> here at all.  Why not just
> 
>     FOR i IN 1..MaxDays LOOP
>         SELECT count(*)
>           INTO r
>           FROM users
>          WHERE lastlogin between current_date - (i-1) * interval ''1 day''
>                              AND current_date - i * interval ''1 day'';
>         RETURN NEXT r;
>     END LOOP;

Thank you.  I had to replace the " with \', but here is what I came 
up with (after adding another item to the SELECT):

CREATE OR REPLACE FUNCTION LoginCounts(int) RETURNS SETOF 
logincounts_record AS '
DECLARE
    MaxDays ALIAS for $1;

    r   logincounts_record%rowtype;
    i   integer;

BEGIN
    raise notice ''MaxDays'';
    FOR i IN 1..MaxDays LOOP
        SELECT 1 AS days,
               count(*) as count
          INTO r
          FROM users
         WHERE lastlogin between current_date - (i-1) * interval \'1 
day\'
                             AND current_date - i     * interval \'1 
day\';

        RETURN NEXT r;
    END LOOP;
    RETURN;
END
'
LANGUAGE plpgsql;

However, the results are confusing.  I'm getting the wrong number of 
parameters.  The value being returned appears to be the value 
supplied.  But the log results show an interesting pattern in the 
number of selects being run.


working-copy.freshports.org=# select count(*) from LoginCounts(1);
NOTICE:  MaxDays
 count
-------
     1
(1 row)

The log says:

2003-09-28 16:01:54 [32813]  LOG:  query: select count(*) from 
LoginCounts(1);
2003-09-28 16:01:54 [32813]  NOTICE:  MaxDays
2003-09-28 16:01:54 [32813]  LOG:  query: select cast($1 as timestamp 
without time zone) - $2;


working-copy.freshports.org=# select count(*) from LoginCounts(2);
NOTICE:  MaxDays
 count
-------
     2
(1 row)

And the log says:

2003-09-28 16:02:04 [32813]  LOG:  query: select count(*) from 
LoginCounts(2);
2003-09-28 16:02:04 [32813]  NOTICE:  MaxDays
2003-09-28 16:02:04 [32813]  LOG:  query: select cast($1 as timestamp 
without time zone) - $2;
2003-09-28 16:02:04 [32813]  LOG:  query: select cast($1 as timestamp 
without time zone) - $2;
2003-09-28 16:02:04 [32813]  LOG:  query: select cast($1 as timestamp 
without time zone) - $2;


The type in question is:


CREATE TYPE logincounts_record AS (
    days            integer,
    count           integer
);
-- 
Dan Langille : http://www.langille.org/

Re:

Tom Lane <tgl@sss.pgh.pa.us> — 2003-09-28T20:14:44Z

"Dan Langille" <dan@langille.org> writes:
> However, the results are confusing.  I'm getting the wrong number of 
> parameters.  The value being returned appears to be the value 
> supplied.  But the log results show an interesting pattern in the 
> number of selects being run.

I dunno where the cast() queries are coming from, but note that they're
not your SELECT.  You are misunderstanding how the code works if you
expect to see query LOG entries from plpgsql queries.  For a
non-EXECUTEd plpgsql command, log_statement will only show the query the
first time it is executed in a session, because that log entry is
generated as a side-effect of parsing and planning.

As a means of tracing the execution of plpgsql functions, log_statement
is pretty worthless :-(.  I would like us to develop a full-up tracing
and debugging facility for plpgsql, but we haven't got it yet.

			regards, tom lane

Re:

Dan Langille <dan@langille.org> — 2003-09-29T14:03:19Z

On 29 Sep 2003 at 10:04, Jean-Luc Lachance wrote:

> Wouldn't:
> 
> insert into r 
> select count(*) 
> from users 
> where date( lastlogin) > current_date - MaxDays * interval '' 1 day''
> group by date( lastlogin);
> 
> be more efficient?


Yes it would, by a factor of 5.

freshports=# explain analyse select * from LoginCounts(3);
                                                    QUERY PLAN
----------------------------------------------------------------------
--------------------------------------------
 Function Scan on logincounts  (cost=0.00..12.50 rows=1000 width=8) 
(actual time=1141.04..1141.06 rows=3 loops=1)
 Total runtime: 1141.13 msec
(2 rows)

freshports=# explain analyse select count(*)
freshports-# from users
freshports-# where date( lastlogin) > current_date - 3 * interval ' 1 
day'
freshports-# group by date( lastlogin);
                                                        QUERY PLAN
----------------------------------------------------------------------
-----------------------------------------------------
 Aggregate  (cost=539.78..552.75 rows=173 width=8) (actual 
time=197.54..198.97 rows=3 loops=1)
   ->  Group  (cost=539.78..548.42 rows=1730 width=8) (actual 
time=196.97..198.43 rows=110 loops=1)
         ->  Sort  (cost=539.78..544.10 rows=1730 width=8) (actual 
time=196.95..197.39 rows=110 loops=1)
               Sort Key: date(lastlogin)
               ->  Seq Scan on users  (cost=0.00..446.75 rows=1730 
width=8) (actual time=0.87..195.38 rows=110 loops=1)
                     Filter: ((date(lastlogin))::timestamp without 
time zone > (('now'::text)::date - '3 days'::interval))
 Total runtime: 199.33 msec
(7 rows)

freshports=#

Thank you.
-- 
Dan Langille : http://www.langille.org/

Re:

Jean-Luc Lachance <jllachan@nsd.ca> — 2003-09-29T14:04:08Z

Wouldn't:

insert into r 
select count(*) 
from users 
where date( lastlogin) > current_date - MaxDays * interval '' 1 day''
group by date( lastlogin);

be more efficient?



Tom Lane wrote:
> 
> Dan Langille <dan@langille.org> writes:
> >          WHERE lastlogin between current_date - interval \''' ||
> > quote_literal(i - 1) || '' days\'
> >                              AND current_date - interval \''' ||
> > quote_literal(i)     || '' days\''';
> 
> IIRC, quote_literal() puts single quotes around its result.  So you have
> too many quotes there.  Given that you know i is an integer, you don't
> really need quote_literal for it.  Actually, you don't need EXECUTE
> here at all.  Why not just
> 
>     FOR i IN 1..MaxDays LOOP
>         SELECT count(*)
>           INTO r
>           FROM users
>          WHERE lastlogin between current_date - (i-1) * interval ''1 day''
>                              AND current_date - i * interval ''1 day'';
>         RETURN NEXT r;
>     END LOOP;
> 
>                         regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings

Re:

Dan Langille <dan@langille.org> — 2003-09-29T21:19:04Z

On 29 Sep 2003 at 10:04, Jean-Luc Lachance wrote:

> Wouldn't:
> 
> insert into r 
> select count(*) 
> from users 
> where date( lastlogin) > current_date - MaxDays * interval '' 1 day''
> group by date( lastlogin);
> 
> be more efficient?


Yes it would, by a factor of 5.

P.S. but it would not show dates for which there are no logins.  The 
above can return zero rows.  The previous example always returns 
MaxDays rows.
-- 
Dan Langille : http://www.langille.org/