Re: [GENERAL] Count(*) throws error

Hi,

I'm using the following statement in my plpgsql function

SELECT INTO no_rows COUNT(*) FROM tbl_concurrent;

I have decalred no_rows int4 and initialized it to zero

Running the function throws the following error:

ERROR: syntax error at or near "(" at character 13
QUERY: SELECT $1 (*) FROM tbl_concurrent
CONTEXT: SQL statement in PL/PgSQL function "sp_insert_tbl_concurrent" near
line 8

If i comment this count(*) line, the error goes.

I don't know why isn't count(*) working

Thanks
Jas

On Jul 11, 2007, at 4:00 PM, Jasbinder Singh Bali wrote:

> Hi,
>
> I'm using the following statement in my plpgsql function
>
> SELECT INTO no_rows COUNT(*) FROM tbl_concurrent;
>
> I have decalred no_rows int4 and initialized it to zero
>
> Running the function throws the following error:
>
> ERROR: syntax error at or near "(" at character 13
> QUERY: SELECT $1 (*) FROM tbl_concurrent
> CONTEXT: SQL statement in PL/PgSQL function
> "sp_insert_tbl_concurrent" near line 8
>
>
> If i comment this count(*) line, the error goes.
>
> I don't know why isn't count(*) working

SELECT COUNT(*) INTO no_rows FROM tbl_concurrent;

Erik Jones

Software Developer | Emma®
erik(at)myemma(dot)com
800.595.4401 or 615.292.5888
615.292.0777 (fax)

Emma helps organizations everywhere communicate & market in style.
Visit us online at http://www.myemma.com

On Jul 11, 4:00 pm, jsb(dot)(dot)(dot)(at)gmail(dot)com ("Jasbinder Singh Bali") wrote:
> I don't know why isn't count(*) working

Works for me:

create table tbl_concurrent(a int);
insert into tbl_concurrent values (1);
insert into tbl_concurrent values (9);
insert into tbl_concurrent values (4);

create or replace function foo() returns int as
$$
declare
no_rows int := 0;
begin
select into no_rows count(*) from tbl_concurrent;
return no_rows;
end;
$$
language plpgsql;

select foo();

foo
-----
3

"Jasbinder Singh Bali" <jsbali(at)gmail(dot)com> writes:
> I'm using the following statement in my plpgsql function

> SELECT INTO no_rows COUNT(*) FROM tbl_concurrent;

> I have decalred no_rows int4 and initialized it to zero

> Running the function throws the following error:

> ERROR: syntax error at or near "(" at character 13
> QUERY: SELECT $1 (*) FROM tbl_concurrent

I'll bet a nickel you have a local variable named "count" in that
function, and plpgsql is blindly trying to substitute its value into
the SQL query. The replacement of "COUNT" by " $1 " in the query
text is the tip-off.

regards, tom lane

On Wed, 2007-07-11 at 17:42 -0400, Tom Lane wrote:
> "Jasbinder Singh Bali" <jsbali(at)gmail(dot)com> writes:
> > I'm using the following statement in my plpgsql function
>
> > SELECT INTO no_rows COUNT(*) FROM tbl_concurrent;
>
> > I have decalred no_rows int4 and initialized it to zero
>
> > Running the function throws the following error:
>
> > ERROR: syntax error at or near "(" at character 13
> > QUERY: SELECT $1 (*) FROM tbl_concurrent
>
> I'll bet a nickel you have a local variable named "count" in that
> function, and plpgsql is blindly trying to substitute its value into
> the SQL query. The replacement of "COUNT" by " $1 " in the query
> text is the tip-off.

I came across a boat load of these the other day. Seems fairly naff that
we substitute variables blindly.

Seems like we could be slightly more friendly without too much bother:
at least only substitute after the VALUES clause in INSERT. We really
shouldn't substitute "var = var" to "$n = $n" either; am I right in
thinking the latter would happen silently and cause potential error?

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

"Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
> Seems like we could be slightly more friendly without too much bother:
> at least only substitute after the VALUES clause in INSERT.

Surely you jest.

regards, tom lane

"Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
> Seems like we could be slightly more friendly without too much bother:

Actually, rather than get into that sort of AI-complete project,
it strikes me that the most useful response is user education. There
ought to be a section in the plpgsql documentation explaining exactly
how the substitution works and showing a couple examples of the pitfalls.
I'll try to draft something up.

regards, tom lane

On Wed, 2007-07-11 at 18:13 -0400, Tom Lane wrote:
> "Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
> > Seems like we could be slightly more friendly without too much bother:
> > at least only substitute after the VALUES clause in INSERT.
>
> Surely you jest.

No. There are a places where parameters clearly aren't allowed, so
making the substitutions in those places can easily be prevented. The
remainder of the problem is as hard as you think, but getting half way
there seems very easy.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> "Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
>> Seems like we could be slightly more friendly without too much bother:
>
> Actually, rather than get into that sort of AI-complete project,

Is it really AI-complete? ISTM the *only* place where a parameter is allowed
is where the parser inserts a ColumnRef node?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

"Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
> On Wed, 2007-07-11 at 18:13 -0400, Tom Lane wrote:
>> "Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
>>> Seems like we could be slightly more friendly without too much bother:
>>> at least only substitute after the VALUES clause in INSERT.
>>
>> Surely you jest.

> No. There are a places where parameters clearly aren't allowed, so
> making the substitutions in those places can easily be prevented. The
> remainder of the problem is as hard as you think, but getting half way
> there seems very easy.

It's not nearly as easy as you think. Even for the limited case of
the column list for an INSERT, consider

DECLARE i int; j int;
...
INSERT INTO mytable (arraycol[i]) VALUES (j);

Considering that the current plpgsql parser doesn't have any knowledge
*at all* of the syntactic structure of individual SQL commands, even
teaching it to recognize an INSERT column list correctly would be a huge
addition of code. The payback for that seems darn small considering
that (1) that's only a small portion of the trouble cases and (2) it is
very easy for users to recognize a failure in this context, whereas many
of the other trouble cases aren't mechanically recognizable at all.

Ultimately we have to do a better job of teaching users to recognize
this issue for themselves; which is why I think it's basically a
documentation problem.

The other problem with trying to inject a small amount of smarts is that
it complicates explaining the system. I think the docs have to explain
*exactly* how the substitution works (which they fail to do right now,
but I intend to make 'em do so today), and putting in any special cases
will make that explanation longer and more confusing.

regards, tom lane

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>> Actually, rather than get into that sort of AI-complete project,

> Is it really AI-complete? ISTM the *only* place where a parameter is allowed
> is where the parser inserts a ColumnRef node?

The first problem is that we have to make the decision far upstream of that.

The second is that whether we *can* insert a parameter symbol is not the
same as whether we *should*. Even if we made that work, we'd only be
fixing the cases that currently result in trivial-for-users-to-recognize
syntactic errors. The deeper semantic errors arising from substituting
a parameter where the user meant to refer to a similarly-named column
can only be dealt with through user education.

regards, tom lane

"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> "Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
>> On Wed, 2007-07-11 at 18:13 -0400, Tom Lane wrote:
>>> "Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
>>>> Seems like we could be slightly more friendly without too much bother:
>>>> at least only substitute after the VALUES clause in INSERT.
>>>
>>> Surely you jest.
>
>> No. There are a places where parameters clearly aren't allowed, so
>> making the substitutions in those places can easily be prevented. The
>> remainder of the problem is as hard as you think, but getting half way
>> there seems very easy.
>
> It's not nearly as easy as you think. Even for the limited case of
> the column list for an INSERT, consider
>
> DECLARE i int; j int;
> ...
> INSERT INTO mytable (arraycol[i]) VALUES (j);

huh, i hadn't actually seen that before -- it doesn't seem very useful. But
I've certainly seen and used "UPDATE SET arraycol[i] = val".

> Considering that the current plpgsql parser doesn't have any knowledge
> *at all* of the syntactic structure of individual SQL commands, even
> teaching it to recognize an INSERT column list correctly would be a huge
> addition of code.

It does eventually call the SQL parser though doesn't it? I was thinking the
way to do it would be to delay substituting the formal parameters to later,
after the parsing.

So instead of substituting them as the tokens are lexed, instead suck in the
tokens, run the parser -- which we currently do anyways just to check the
syntax -- then walk the tree looking for ColumnRefs where the name matches a
variable name. Then keep around that parse tree instead of just the series of
lex tokens to later call analyze on and execute.

> The other problem with trying to inject a small amount of smarts is that
> it complicates explaining the system.

Well that's quite true, but then that's the basic difficulty with any part of
plpgsql.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> So instead of substituting them as the tokens are lexed, instead suck in the
> tokens, run the parser -- which we currently do anyways just to check the
> syntax -- then walk the tree looking for ColumnRefs where the name matches a
> variable name. Then keep around that parse tree instead of just the series of
> lex tokens to later call analyze on and execute.

Hmm. That sounds cool but I think it actually has a pretty substantial
disadvantage --- there is then no easily user-readable representation of
the query that shows *which* occurrences got substituted. With the
textual replacement method you have a string you can look at, though
it's true that we don't always show it to the user if we are not aware
there's a problem.

Again, I'm trying to look at the big picture of both syntactic and
semantic errors. If we solve only the syntactic end of it I think we'd
actually be worse off, because then users would be even more lost when
they hit a semantic error (unwanted substitution).

regards, tom lane

Tom Lane wrote:
> Again, I'm trying to look at the big picture of both syntactic and
> semantic errors. If we solve only the syntactic end of it I think we'd
> actually be worse off, because then users would be even more lost when
> they hit a semantic error (unwanted substitution).

The only real solution is to have some clear syntactic sugar denoting
where variables are to be interpolated:
SELECT * FROM foo WHERE (bar + 1) = bar -- Which is the var?
SELECT * FROM foo WHERE (bar + 1) = $bar -- OK
SELECT * FROM foo WHERE (bar + 1) = {bar} -- OK
It's not clear to plpgsql because it's *not clear*.

In any other namespace-conflict situation I can think of it's always
inner-definition-is-visible. This would of course solve the problem, but
only by preventing you from substituting in variables that conflict
with columns. Unless you generate a warning at function compile-time
that doesn't seem much better.

--
Richard Huxton
Archonet Ltd