Thread: Prepare/Execute silently discards prohibited ORDER BY values
Tested On: 9.4.1, 9.3.6
Severity: minor
Summary: PREPARE/EXECUTE appears to silently discard ORDER BY parameters.
josh=# \d test
Table "public.test"
Column | Type | Modifiers
--------+------+-----------
test | text |
josh=# insert into test values ('test1'),('test9'),('test3'),('test2');
INSERT 0 4
josh=# prepare foo as select * from test order by $1;
PREPARE
josh=# execute foo('test');
test
-------
test1
test9
test3
test2
(4 rows)
josh=# explain analyze
josh-# execute foo('test');
QUERY PLAN
---------------------------------------------------------------------------------------------------
Seq Scan on test (cost=0.00..23.10 rows=1310 width=32) (actual
time=0.007..0.007 rows=4 loops=1)
Execution time: 0.026 ms
(2 rows)
What appears to be happening is that the prohibited parameter for ORDER
BY is being silently discarded during EXECUTE. At first I thought it
might just be doing ORDER BY 'test' in the background, but that's not it:
josh=# select * from test order by 'test';
ERROR: non-integer constant in ORDER BY
LINE 1: select * from test order by 'test';
josh=# execute foo(1);
test
-------
test1
test9
test3
test2
(4 rows)
josh=# select * from pg_prepared_statements ;
name | statement |
prepare_time | parameter_types | from_sql
------+------------------------------------------------+-------------------------------+-----------------+----------
foo | prepare foo as select * from test order by $1; | 2015-05-11
16:52:55.369479-07 | {text} | t
(1 row)
So something else is happening here. What should probably be happening
is that PREPARE should throw an error if it gets a parameter in the
ORDER BY clause.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
Josh Berkus <josh@agliodbs.com> writes:
> Tested On: 9.4.1, 9.3.6
> Severity: minor
> Summary: PREPARE/EXECUTE appears to silently discard ORDER BY parameters.
> josh=# prepare foo as select * from test order by $1;
> PREPARE
I don't see anything wrong with this. Ordering by a provably constant
expression is a no-op, not an error.
> What appears to be happening is that the prohibited parameter for ORDER
> BY is being silently discarded during EXECUTE. At first I thought it
> might just be doing ORDER BY 'test' in the background, but that's not it:
> josh=# select * from test order by 'test';
> ERROR: non-integer constant in ORDER BY
This error is purely a syntactic restriction, not a semantic one.
There's nothing that will stop you from ordering by, say, "cos(0)";
and the planner will throw that away too.
regards, tom lane
On 05/11/2015 05:18 PM, Tom Lane wrote: > Josh Berkus <josh@agliodbs.com> writes: >> Tested On: 9.4.1, 9.3.6 >> Severity: minor >> Summary: PREPARE/EXECUTE appears to silently discard ORDER BY parameters. > >> josh=# prepare foo as select * from test order by $1; >> PREPARE > > I don't see anything wrong with this. Ordering by a provably constant > expression is a no-op, not an error. > >> What appears to be happening is that the prohibited parameter for ORDER >> BY is being silently discarded during EXECUTE. At first I thought it >> might just be doing ORDER BY 'test' in the background, but that's not it: > >> josh=# select * from test order by 'test'; >> ERROR: non-integer constant in ORDER BY > > This error is purely a syntactic restriction, not a semantic one. > There's nothing that will stop you from ordering by, say, "cos(0)"; > and the planner will throw that away too. Ah, ok. The problem is that in the SELECT case, 'test' isn't typed, so the parser is trying to evaluate it and fails? That makes sense. josh=# select * from test order by 'test'::TEXT; test ------- test1 test9 test3 test2 (4 rows) Issue closed. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh Berkus <josh@agliodbs.com> writes:
> On 05/11/2015 05:18 PM, Tom Lane wrote:
>> Josh Berkus <josh@agliodbs.com> writes:
>>> josh=# select * from test order by 'test';
>>> ERROR: non-integer constant in ORDER BY
>> This error is purely a syntactic restriction, not a semantic one.
>> There's nothing that will stop you from ordering by, say, "cos(0)";
>> and the planner will throw that away too.
> Ah, ok. The problem is that in the SELECT case, 'test' isn't typed, so
> the parser is trying to evaluate it and fails? That makes sense.
Well, not quite. The core problem is that SQL92 said that "ORDER BY n"
(where "n" could only be an integer constant) means "order by the N'th
output column" ... and then SQL99 forgot about that altogether, and
defined the entirely more sensible rule that ORDER BY items are just
expressions that have their face value. We try to support both of those
cases, both for backwards compatibility and because ORDER BY n (also
GROUP BY n) is such a damn handy abbreviation so much of the time.
Somewhere along the line we decided that "ORDER BY non-integer-constant"
was too close to the boundary line between those two interpretations, so
it would be better to reject it and make you use a less ambiguous syntax.
I'm too lazy to go digging in the archives for that discussion (it was
quite a few years back, though). But that's why you're seeing a syntax
failure for "ORDER BY 'test'". We could certainly make that case do
something else if we wanted ... but I'm not sure it'd be an improvement.
regards, tom lane
On 05/11/2015 08:35 PM, Tom Lane wrote: >> Ah, ok. The problem is that in the SELECT case, 'test' isn't typed, so >> > the parser is trying to evaluate it and fails? That makes sense. > Well, not quite. The core problem is that SQL92 said that "ORDER BY n" > (where "n" could only be an integer constant) means "order by the N'th > output column" ... and then SQL99 forgot about that altogether, and > defined the entirely more sensible rule that ORDER BY items are just > expressions that have their face value. We try to support both of those > cases, both for backwards compatibility and because ORDER BY n (also > GROUP BY n) is such a damn handy abbreviation so much of the time. > > Somewhere along the line we decided that "ORDER BY non-integer-constant" > was too close to the boundary line between those two interpretations, so > it would be better to reject it and make you use a less ambiguous syntax. > I'm too lazy to go digging in the archives for that discussion (it was > quite a few years back, though). But that's why you're seeing a syntax > failure for "ORDER BY 'test'". We could certainly make that case do > something else if we wanted ... but I'm not sure it'd be an improvement. Well, the fact that: ORDER BY 'test' ... errrors, whereas ORDER BY 'test'::TEXT ... does not is a very small POLS violation. I was not the only one confused by it; pitching this on IRC, several postgres hackers and advanced users thought it was a bug. On the other hand, it's also not exactly breaking anyone's stuff once you understand what it's doing. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh Berkus <josh@agliodbs.com> writes:
> On 05/11/2015 08:35 PM, Tom Lane wrote:
>> Somewhere along the line we decided that "ORDER BY non-integer-constant"
>> was too close to the boundary line between those two interpretations, so
>> it would be better to reject it and make you use a less ambiguous syntax.
> Well, the fact that:
> ORDER BY 'test'
> ... errrors, whereas
> ORDER BY 'test'::TEXT
> ... does not is a very small POLS violation.
Agreed; I think the reason why we disallow this case is not so much
string literals as numeric literals. It would be an even bigger POLA
violation if "ORDER BY 1" and "ORDER BY 1.0" were both accepted and did
fundamentally different things.
regards, tom lane
Tom Lane wrote, On 2015-05-14 03:05:
> Agreed; I think the reason why we disallow this case is not so much
> string literals as numeric literals. It would be an even bigger POLA
> violation if "ORDER BY 1" and "ORDER BY 1.0" were both accepted and did
> fundamentally different things.
If that helps, I don't think of that as violating the POLA
significantly. This is IMO a much worse violation:
=$ WITH test AS (SELECT 1 UNION ALL SELECT 0) SELECT * FROM test ORDER BY 1;
?column?
----------
0
1
(2 rows)
=$ WITH test AS (SELECT 1 UNION ALL SELECT 0) SELECT * FROM test ORDER
BY 1::integer;
?column?
----------
1
0
(2 rows)
especially since the latter ORDER BY clause is more likely to be the
result of a dumb automatic query generator that appends types to every
expression. But I, as an user, understand and accept that appending a
type changes the meaning of the ORDER BY.