Thread: BUG #8633: Assigning to a variable named "current_time" gives wrong output

The following bug has been logged on the website:

Bug reference:      8633
Logged by:          Michael Lancaster
Email address:      michael.lancaster@exa-networks.co.uk
PostgreSQL version: 9.1.10
Operating system:   Linux ubuntu 3.5.0-42-generic #65~precise1-Ubuntu
Description:



Reading the value from a variable named 'current_time' that has a value
assigned to it then assigns the value of the function 'current_time' rather
than reading the value that was previously assigned.
Changing the variable name to anything else - current_time_value for example
- fixes the issue.
Table schema is not included as the function works without issue in psql
version 8.4.7.


------------------------------------------------------------


create type period_call_count_type as (
    start_time timestamp,
    end_time timestamp,
    max_count integer
);


------------------------------------------------------------


create or replace function call_count (text, date, date=null) returns setof
period_call_count_type as $$
DECLARE
    customer_value alias for $1;
    start_date alias for $2;
    end_date alias for $3;


    start_time timestamp;
    end_time timestamp;
    current_time timestamp;
    threshold_time timestamp;


    resolution_value interval;
    max_count_value integer;
    current_count_value integer;


    edge_value period_edge_type;
    result period_call_count_type;
BEGIN
    raise warning 'start_date: %',start_date;


    resolution_value := '10 minutes'::interval;
    raise warning 'resolution_value: %',resolution_value;


    current_time := start_date::timestamp;
    raise warning 'current_time: %',current_time;


    start_time := start_date::timestamp;
    raise warning 'start_time: %',start_time;


    threshold_time := current_time + resolution_value;




    if end_date is not null then
        end_time := end_date + '1 day'::interval;
    else
        end_time := start_date + '1 day'::interval;
    end if;




    max_count_value := 0;
    current_count_value := 0;


    for edge_value in
        select    event, event_time
        from    detect_call_edges(customer_value, start_time, end_time)
        order    by event_time nulls first, event
    loop
        while edge_value.event_time >= threshold_time loop


            result.start_time := current_time;
            result.end_time := threshold_time;
            result.max_count := max_count_value;


            current_time = threshold_time;
            threshold_time = threshold_time + resolution_value;
            max_count_value := current_count_value;


            return next result;
        end loop;


        if edge_value.event = 'start'::text then
            current_count_value := current_count_value + 1;
            if current_count_value > max_count_value then
                max_count_value = current_count_value;
            end if;


        elsif edge_value.event = 'stop'::text then
            current_count_value := current_count_value - 1;
            if current_count_value < 0 then
                raise exception 'concurrent call count drops below 0';
            end if;
        end if;
    end loop;




    result.start_time := current_time;
    result.end_time := current_time + resolution_value;
    result.max_count := max_count_value;
    return next result;




    result.max_count := 0;
    while result.end_time < end_time loop
        result.start_time := result.start_time + resolution_value;
        result.end_time := result.end_time + resolution_value;
        return next result;
    end loop;




    return;
END
$$ language plpgsql security definer;




COMMIT;


-------------------------------------------------------------------------


PSQL QUERY:


select * from call_count('exa','2013-10-01','2013-10-31');


CONSOLE OUTPUT:


select * from call_count('exa','2013-10-01','2013-10-31');


WARNING:  01000: start_date: 2013-10-01
LOCATION:  exec_stmt_raise, pl_exec.c:2799
WARNING:  01000: resolution_value: 00:10:00
LOCATION:  exec_stmt_raise, pl_exec.c:2799
WARNING:  01000: current_time: 15:06:43.601688+00
LOCATION:  exec_stmt_raise, pl_exec.c:2799
WARNING:  01000: start_time: 2013-10-01 00:00:00
LOCATION:  exec_stmt_raise, pl_exec.c:2799
ERROR:  22007: invalid input syntax for type timestamp:
"15:16:43.601688+00"
CONTEXT:  PL/pgSQL function "call_count" line 30 at assignment
LOCATION:  DateTimeParseError, datetime.c:3565


EXPECTED OUTPUT:


The assignment for current_time and start_time should both be the same, a
timestamp of start_date, however current_time is getting the current time
rather than its assigned value.

On Wed, Nov 27, 2013 at 7:49 AM, <michael.lancaster@exa-networks.co.uk>wrote:

>
> Reading the value from a variable named 'current_time' that has a value
> assigned to it then assigns the value of the function 'current_time' rather
> than reading the value that was previously assigned.
>

I don't think this is a bug, "current_time" is a reserved word. See
http://www.postgresql.org/docs/current/static/sql-keywords-appendix.html

bricklen wrote
> On Wed, Nov 27, 2013 at 7:49 AM, <

> michael.lancaster@.co

> >wrote:
>
>>
>> Reading the value from a variable named 'current_time' that has a value
>> assigned to it then assigns the value of the function 'current_time'
>> rather
>> than reading the value that was previously assigned.
>>
>
> I don't think this is a bug, "current_time" is a reserved word. See
> http://www.postgresql.org/docs/current/static/sql-keywords-appendix.html

Agreed; though I am curious why this does not throw an error during the
declaration or assignment. A spurious parser error would be welcomed
compared to silently ignoring the requested action.

David J.




--
View this message in context:
http://postgresql.1045698.n5.nabble.com/BUG-8633-Assigning-to-a-variable-named-current-time-gives-wrong-output-tp5780601p5780604.html
Sent from the PostgreSQL - bugs mailing list archive at Nabble.com.

David Johnston <polobo@yahoo.com> writes:
> bricklen wrote
>> I don't think this is a bug, "current_time" is a reserved word. See
>> http://www.postgresql.org/docs/current/static/sql-keywords-appendix.html

> Agreed; though I am curious why this does not throw an error during the
> declaration or assignment. A spurious parser error would be welcomed
> compared to silently ignoring the requested action.

Well, it's not "ignoring the action", it's just that the keyword meaning
now takes precedence in expressions.  We changed that in 9.0 I believe.

We could possibly throw an error if you use a SQL reserved word in a
declaration without double-quoting it.  That wouldn't be a complete fix,
because if you did that and then forgot to double-quote the name in
expressions, it'd still do the wrong thing.  But this might at least
help you remember you need to do that.

I think though that at one time it was considered a feature that we
didn't insist on double-quoting plpgsql variable names unnecessarily.
Don't know if changing this would be a net improvement for everyone.

            regards, tom lane

Tom Lane-2 wrote
> David Johnston <

> polobo@

> > writes:
>> bricklen wrote
>>> I don't think this is a bug, "current_time" is a reserved word. See
>>> http://www.postgresql.org/docs/current/static/sql-keywords-appendix.html
>
>> Agreed; though I am curious why this does not throw an error during the
>> declaration or assignment. A spurious parser error would be welcomed
>> compared to silently ignoring the requested action.
>
> Well, it's not "ignoring the action", it's just that the keyword meaning
> now takes precedence in expressions.  We changed that in 9.0 I believe.
>
> We could possibly throw an error if you use a SQL reserved word in a
> declaration without double-quoting it.  That wouldn't be a complete fix,
> because if you did that and then forgot to double-quote the name in
> expressions, it'd still do the wrong thing.  But this might at least
> help you remember you need to do that.
>
> I think though that at one time it was considered a feature that we
> didn't insist on double-quoting plpgsql variable names unnecessarily.
> Don't know if changing this would be a net improvement for everyone.

If we had done this when all of the other pl/pgsql variable-name changes
were put in place that would have been nice but to do it stand-alone
definitely has a higher hurdle to clear.

At this point I'd rather leave this to a linter / static-analyzer rather
than affect existing code unconditionally.

David J.




--
View this message in context:
http://postgresql.1045698.n5.nabble.com/BUG-8633-Assigning-to-a-variable-named-current-time-gives-wrong-output-tp5780601p5780626.html
Sent from the PostgreSQL - bugs mailing list archive at Nabble.com.

2013/11/27 David Johnston <polobo@yahoo.com>

> Tom Lane-2 wrote
> > David Johnston <
>
> > polobo@
>
> > > writes:
> >> bricklen wrote
> >>> I don't think this is a bug, "current_time" is a reserved word. See
> >>>
> http://www.postgresql.org/docs/current/static/sql-keywords-appendix.html
> >
> >> Agreed; though I am curious why this does not throw an error during the
> >> declaration or assignment. A spurious parser error would be welcomed
> >> compared to silently ignoring the requested action.
> >
> > Well, it's not "ignoring the action", it's just that the keyword meaning
> > now takes precedence in expressions.  We changed that in 9.0 I believe.
> >
> > We could possibly throw an error if you use a SQL reserved word in a
> > declaration without double-quoting it.  That wouldn't be a complete fix,
> > because if you did that and then forgot to double-quote the name in
> > expressions, it'd still do the wrong thing.  But this might at least
> > help you remember you need to do that.
> >
> > I think though that at one time it was considered a feature that we
> > didn't insist on double-quoting plpgsql variable names unnecessarily.
> > Don't know if changing this would be a net improvement for everyone.
>
> If we had done this when all of the other pl/pgsql variable-name changes
> were put in place that would have been nice but to do it stand-alone
> definitely has a higher hurdle to clear.
>
> At this point I'd rather leave this to a linter / static-analyzer rather
> than affect existing code unconditionally.
>

this tool exists https://github.com/okbob/plpgsql_lint and should be
enhanced for your issue

Regards

Pavel




>
> David J.
>
>
>
>
> --
> View this message in context:
>
http://postgresql.1045698.n5.nabble.com/BUG-8633-Assigning-to-a-variable-named-current-time-gives-wrong-output-tp5780601p5780626.html
> Sent from the PostgreSQL - bugs mailing list archive at Nabble.com.
>
>
> --
> Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs
>