Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

Started by Regina Obealmost 9 years ago29 messageshackers
Jump to latest
#1Regina Obe
lr@pcorp.us

Did something change with how exclusion constraints are handled? I'm

trying to troubleshoot a regression we are having with PostGIS raster
support.

As best I can guess, it's because exclusion constraints that used to work

in past versions are failing in PostgreSQL 10 with an error something like

this:

ERROR: conflicting key value violates exclusion constraint

"enforce_spatially_unique_test_raster_columns_rast"

ERROR: new row for relation "test_raster_columns" violates check

constraint "enforce_coverage_tile_rast"

Unfortunately I don't know how long this has been an issue since we had an

earlier test failing preventing the raster ones from being tested.

Thanks,
Regina

I figured out the culprit was the change in CASE WHEN behavior with set
returning functions

Had a criteria something of the form:

CASE WHEN some_condition_dependent_on_sometable_that_resolves_to_false THEN
(regexp_matches(...))[1] ELSE ... END
FROM sometable;

One thing that seems a little odd to me is why these return a record

SELECT CASE WHEN strpos('ABC', 'd') > 1 THEN (regexp_matches('a (b) c',
'd'))[1] ELSE 'a' END;

SELECT CASE WHEN false THEN (regexp_matches('a (b) c', 'd'))[1] ELSE 'a' END
FROM pg_tables;

And this doesn't - I'm guessing it has to do with this being a function of
the value of table, but it seems unintuitive
From a user perspective.

SELECT CASE WHEN strpos(f.tablename, 'ANY (ARRAY[') > 1 THEN
(regexp_matches('a (b) c', 'd'))[1] ELSE 'a' END
FROM pg_tables AS f;

Pre-PostgreSQL 10 this would return a row for each record in pg_tables

Thanks,
Regina

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Regina Obe (#1)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

"Regina Obe" <lr@pcorp.us> writes:

I figured out the culprit was the change in CASE WHEN behavior with set
returning functions
Had a criteria something of the form:
CASE WHEN some_condition_dependent_on_sometable_that_resolves_to_false THEN
(regexp_matches(...))[1] ELSE ... END
FROM sometable;

You might want to consider changing such usages to use regexp_match()
instead of regexp_matches().

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Regina Obe
lr@pcorp.us
In reply to: Tom Lane (#2)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

"Regina Obe" <lr@pcorp.us> writes:

I figured out the culprit was the change in CASE WHEN behavior with
set returning functions Had a criteria something of the form:
CASE WHEN some_condition_dependent_on_sometable_that_resolves_to_false
THEN (regexp_matches(...))[1] ELSE ... END FROM sometable;

You might want to consider changing such usages to use regexp_match()

instead of regexp_matches().

regards, tom lane

Thanks. I ended up swapping out with substring which was a bit shorter than
regexp_match()[].

But I've got similar problems with PostGIS topology logic and the easiest
change to make was take advantage
of the fact that you guys are treating CASE constant ... THEN SRF ...

Differently

Than

CASE not_constant_based_on_table_value THEN SRF ..

So I switched those to constant checks. This feels a little dirty and
fragile to me though.

Is this behavior going to stay or change?

It seems inconsistent from a user perspective that

CASE constant .... == short-circuit skipping over SRFs that may otherwise
fail

While

CASE not_constant_table_dependent doesn't short-circuit.

I can understand the motive behind it, it just feels a little inconsistent
from an end-user POV.

Thanks,
Regina

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Regina Obe (#3)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

"Regina Obe" <lr@pcorp.us> writes:

Is this behavior going to stay or change?
It seems inconsistent from a user perspective that
CASE constant .... == short-circuit skipping over SRFs that may otherwise
fail
While
CASE not_constant_table_dependent doesn't short-circuit.
I can understand the motive behind it, it just feels a little inconsistent
from an end-user POV.

After thinking about this for awhile, I agree that what we've got now
isn't too satisfactory. We can make an analogy between SRFs and
aggregate functions: both of them look like simple function calls
syntactically, but they have global effects on the semantics of the
query, particularly on how many rows are returned.

In the case of aggregates, there is long-standing precedent that we
can optimize away individual aggregate calls but the query semantics
do not change, ie you get one output row (or one per GROUP BY group)
even if every last aggregate call disappears due to CASE simplification.
The same was true for deletion of SRFs by CASE-simplification before
v10, but now we've broken that, which seems like a clear bug.

I think it would be possible to teach eval_const_expressions that
it must not discard CASE/COALESCE subexpressions that contain SRFs,
which would preserve the rule that expression simplification doesn't
change the query semantics.

Another possibility is to say that we've broken this situation
irretrievably and we should start throwing errors for SRFs in
places where they'd be conditionally evaluated. That's not real
nice perhaps, but it's better than the way things are right now.

Thoughts?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#4)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

Hi,

On 2017-05-28 14:03:26 -0400, Tom Lane wrote:

I think it would be possible to teach eval_const_expressions that
it must not discard CASE/COALESCE subexpressions that contain SRFs,
which would preserve the rule that expression simplification doesn't
change the query semantics.

That sounds like a good idea. Do you want to write up a patch, or
should I? I can, but I'd want to finish the walsender panic and other
signal handling stuff first (mostly waiting for review for now).

Another possibility is to say that we've broken this situation
irretrievably and we should start throwing errors for SRFs in
places where they'd be conditionally evaluated. That's not real
nice perhaps, but it's better than the way things are right now.

I'd be ok with that too, but I don't really see a strong need so far.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#5)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

Andres Freund <andres@anarazel.de> writes:

On 2017-05-28 14:03:26 -0400, Tom Lane wrote:

I think it would be possible to teach eval_const_expressions that
it must not discard CASE/COALESCE subexpressions that contain SRFs,
which would preserve the rule that expression simplification doesn't
change the query semantics.

That sounds like a good idea. Do you want to write up a patch, or
should I? I can, but I'd want to finish the walsender panic and other
signal handling stuff first (mostly waiting for review for now).

I think you've got enough on your plate. I can take care of whatever
we decide to do here. What I was looking for was opinions on which
way to address it.

Another possibility is to say that we've broken this situation
irretrievably and we should start throwing errors for SRFs in
places where they'd be conditionally evaluated. That's not real
nice perhaps, but it's better than the way things are right now.

I'd be ok with that too, but I don't really see a strong need so far.

The argument for this way is basically that it's better to break
apps visibly than silently. The behavior for SRF-inside-CASE is
not going to be the same as before even if we implement the fix
I suggest above, and it's arguable that this new behavior is not
at all intuitive.

I'm not really sure which way to jump, which is why I was hoping
for some discussion here.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#6)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

Hi,

On 2017-06-02 22:53:00 -0400, Tom Lane wrote:

I think you've got enough on your plate. I can take care of whatever
we decide to do here.

Thanks.

Andres Freund <andres@anarazel.de> writes:

Another possibility is to say that we've broken this situation
irretrievably and we should start throwing errors for SRFs in
places where they'd be conditionally evaluated. That's not real
nice perhaps, but it's better than the way things are right now.

I'd be ok with that too, but I don't really see a strong need so far.

The argument for this way is basically that it's better to break
apps visibly than silently.

Right, I got that.

The behavior for SRF-inside-CASE is
not going to be the same as before even if we implement the fix
I suggest above, and it's arguable that this new behavior is not
at all intuitive.

Yea, I'm not a big fan of the either the pre v10 or the v10 behaviour of
SRFs inside coalesce/case. Neither is really resonable imo - I'm not
sure a reasonable behaviour even exists. IIRC I'd argued in the
original SRF thread that we should just throw an error, and I think we'd
concluded that we'd not do so for now.

I'm not really sure which way to jump, which is why I was hoping
for some discussion here.

There not really being an intuitive behaviour seems to be a bit of a
reason to disallow. Another argument that I can see is that it'll be
easier to allow it again later, than to do the reverse. But I think the
new behaviour can also be useful, and I suspect not that many people
will hit this...

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Mark Dilger
mark.dilger@enterprisedb.com
In reply to: Andres Freund (#7)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

On Jun 2, 2017, at 8:11 PM, Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2017-06-02 22:53:00 -0400, Tom Lane wrote:

I think you've got enough on your plate. I can take care of whatever
we decide to do here.

Thanks.

Andres Freund <andres@anarazel.de> writes:

Another possibility is to say that we've broken this situation
irretrievably and we should start throwing errors for SRFs in
places where they'd be conditionally evaluated. That's not real
nice perhaps, but it's better than the way things are right now.

I'd be ok with that too, but I don't really see a strong need so far.

The argument for this way is basically that it's better to break
apps visibly than silently.

Right, I got that.

The behavior for SRF-inside-CASE is
not going to be the same as before even if we implement the fix
I suggest above, and it's arguable that this new behavior is not
at all intuitive.

Yea, I'm not a big fan of the either the pre v10 or the v10 behaviour of
SRFs inside coalesce/case. Neither is really resonable imo - I'm not
sure a reasonable behaviour even exists. IIRC I'd argued in the
original SRF thread that we should just throw an error, and I think we'd
concluded that we'd not do so for now.

I am trying to get my head around the type of query you and Tom
are discussing. When you say you are unsure a reasonable behavior
even exists, are you saying such queries have no intuitive meaning?

Can you give an example of such a query which has no intuitive
meaning? Perhaps I am not thinking about the right kind of queries.
I have been thinking about examples like:

SELECT x, CASE WHEN y THEN generate_series(1,z) ELSE 5 END
FROM table_with_columns_x_and_y_and_z;

Which to me gives 'z' output rows per table row where y is true, and
one output row per table row where y is false. That could be changed
with an aggregate function such as:

SELECT x, CASE WHEN y THEN SUM(generate_series(1,z)) ELSE 5 END
FROM table_with_columns_x_and_y;

Which to me gives one output row per table row regardless of whether y
is true.

Thanks, and my apologies if I am merely lacking sufficient imagination to
think of a proper example.

Mark Dilger

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Mark Dilger
mark.dilger@enterprisedb.com
In reply to: Mark Dilger (#8)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

On Jun 4, 2017, at 11:55 AM, Mark Dilger <hornschnorter@gmail.com> wrote:

SELECT x, CASE WHEN y THEN SUM(generate_series(1,z)) ELSE 5 END
FROM table_with_columns_x_and_y;

Sorry, this table is supposed to be the same as the previous one,

table_with_columns_x_and_y_and_z

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Andres Freund
andres@anarazel.de
In reply to: Mark Dilger (#8)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

Hi Mark,

On 2017-06-04 11:55:03 -0700, Mark Dilger wrote:

Yea, I'm not a big fan of the either the pre v10 or the v10 behaviour of
SRFs inside coalesce/case. Neither is really resonable imo - I'm not
sure a reasonable behaviour even exists. IIRC I'd argued in the
original SRF thread that we should just throw an error, and I think we'd
concluded that we'd not do so for now.

I am trying to get my head around the type of query you and Tom
are discussing. When you say you are unsure a reasonable behavior
even exists, are you saying such queries have no intuitive meaning?

I'm not saying that there aren't some cases where it's intuitive, but
there's definitely lots that don't have intuitive meaning. Especially
when there are multiple SRFs in the same targetlist.

Do I understand correctly that you're essentially advocating for the <
v10 behaviour? It'd be nontrivial to implement that, without loosing
the significant benefits (Significantly slower, higher code complexity,
weird behaviour around multiple SRFs) gained by removing the previous
implementation. I'd really like to see some examples of when all of
this is useful - I've yet to see a realistic one that's not just as
easily written differently

Can you give an example of such a query which has no intuitive
meaning? Perhaps I am not thinking about the right kind of queries.
I have been thinking about examples like:

SELECT x, CASE WHEN y THEN generate_series(1,z) ELSE 5 END
FROM table_with_columns_x_and_y_and_z;

Which to me gives 'z' output rows per table row where y is true, and
one output row per table row where y is false.

Try any query that has one SRF outside of the CASE, and one inside. In
the old behaviour that'll make the total number of rows returned nearly
undeterministic because of the least-common-multiple behaviour.

That could be changed with an aggregate function such as:
SELECT x, CASE WHEN y THEN SUM(generate_series(1,z)) ELSE 5 END
FROM table_with_columns_x_and_y;

That query doesn't work. First off, aggregates don't take set arguments
(neither in old nor new releases), secondly aggregates are evaluated
independently of CASE/COALESCE statements, thirdly you're missing group
bys. Those all are independent of the v10 changes.

Thanks, and my apologies if I am merely lacking sufficient imagination to
think of a proper example.

Might be worthwhile to reread the thread about the SRF reimplementation.

/messages/by-id/20160822214023.aaxz5l4igypowyri@alap3.anarazel.de

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Mark Dilger
mark.dilger@enterprisedb.com
In reply to: Andres Freund (#10)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

On Jun 4, 2017, at 12:35 PM, Andres Freund <andres@anarazel.de> wrote:

Hi Mark,

On 2017-06-04 11:55:03 -0700, Mark Dilger wrote:

Yea, I'm not a big fan of the either the pre v10 or the v10 behaviour of
SRFs inside coalesce/case. Neither is really resonable imo - I'm not
sure a reasonable behaviour even exists. IIRC I'd argued in the
original SRF thread that we should just throw an error, and I think we'd
concluded that we'd not do so for now.

I am trying to get my head around the type of query you and Tom
are discussing. When you say you are unsure a reasonable behavior
even exists, are you saying such queries have no intuitive meaning?

I'm not saying that there aren't some cases where it's intuitive, but
there's definitely lots that don't have intuitive meaning. Especially
when there are multiple SRFs in the same targetlist.

Do I understand correctly that you're essentially advocating for the <
v10 behaviour?

No, I'm not advocating either way. I merely wanted to know which queries
you and Tom were talking about.

It'd be nontrivial to implement that, without loosing
the significant benefits (Significantly slower, higher code complexity,
weird behaviour around multiple SRFs) gained by removing the previous
implementation. I'd really like to see some examples of when all of
this is useful - I've yet to see a realistic one that's not just as
easily written differently

Can you give an example of such a query which has no intuitive
meaning? Perhaps I am not thinking about the right kind of queries.
I have been thinking about examples like:

SELECT x, CASE WHEN y THEN generate_series(1,z) ELSE 5 END
FROM table_with_columns_x_and_y_and_z;

Which to me gives 'z' output rows per table row where y is true, and
one output row per table row where y is false.

Try any query that has one SRF outside of the CASE, and one inside. In
the old behaviour that'll make the total number of rows returned nearly
undeterministic because of the least-common-multiple behaviour.

That could be changed with an aggregate function such as:
SELECT x, CASE WHEN y THEN SUM(generate_series(1,z)) ELSE 5 END
FROM table_with_columns_x_and_y;

That query doesn't work. First off, aggregates don't take set arguments
(neither in old nor new releases), secondly aggregates are evaluated
independently of CASE/COALESCE statements, thirdly you're missing group
bys. Those all are independent of the v10 changes.

Sorry, I was not clear. What I meant to get at was that if you remove from the
executor all support for SRFs inside case statements, you might foreclose the option
of extending the syntax at some later date to allow aggregates over SRFs. I'm
not saying that this works currently, but in principle if you allowed that SUM() that
I put up there, you'd get back exactly one row from it, same as you get from the
ELSE clause. That would seem to solve the problem without going so far as
completely disallowing the SRF altogether. The reasons for not putting a GROUP BY
clause in the example are (a) I don't know where it would go, syntactically, and (b)
it would defeat the purpose, because once you are grouping, you again have the
possibility of getting more than one row.

I'm not advocating implementing this; I'm just speculating about possible future
features in which SRFs inside a case statement might be allowed.

Thanks, and my apologies if I am merely lacking sufficient imagination to
think of a proper example.

Might be worthwhile to reread the thread about the SRF reimplementation.

/messages/by-id/20160822214023.aaxz5l4igypowyri@alap3.anarazel.de

This helps, thanks!

Mark Dilger

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Andres Freund
andres@anarazel.de
In reply to: Mark Dilger (#11)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

On 2017-06-04 14:16:14 -0700, Mark Dilger wrote:

Sorry, I was not clear. What I meant to get at was that if you remove from the
executor all support for SRFs inside case statements, you might foreclose the option
of extending the syntax at some later date to allow aggregates over
SRFs.

Seems very unlikely that we'd ever want to do that. The right way to do
this is to simply move the SRF into the from list. Having the executor
support arbitrary sources of tuples would just complicate and slow down
already complicated and slow code...

I'm
not saying that this works currently, but in principle if you allowed that SUM() that
I put up there, you'd get back exactly one row from it, same as you get from the
ELSE clause. That would seem to solve the problem without going so far as
completely disallowing the SRF altogether.

But what would the benefit be?

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13Mark Dilger
mark.dilger@enterprisedb.com
In reply to: Andres Freund (#12)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

On Jun 4, 2017, at 2:19 PM, Andres Freund <andres@anarazel.de> wrote:

On 2017-06-04 14:16:14 -0700, Mark Dilger wrote:

Sorry, I was not clear. What I meant to get at was that if you remove from the
executor all support for SRFs inside case statements, you might foreclose the option
of extending the syntax at some later date to allow aggregates over
SRFs.

Seems very unlikely that we'd ever want to do that. The right way to do
this is to simply move the SRF into the from list. Having the executor
support arbitrary sources of tuples would just complicate and slow down
already complicated and slow code...

I'm
not saying that this works currently, but in principle if you allowed that SUM() that
I put up there, you'd get back exactly one row from it, same as you get from the
ELSE clause. That would seem to solve the problem without going so far as
completely disallowing the SRF altogether.

But what would the benefit be?

In my example, the aggregate function is taking a column from the table as
an argument, so the output of the aggregate function needs to be computed per row,
not just once. And if the function is expensive, or has side-effects, you might
only want it to execute for those rows where the CASE statement is true, rather
than for all of them. You may get that same behavior using lateral or some such,
I'm uncertain, but in a complicated CASE statement, it be more straightforward
to write something like:

SELECT
CASE
WHEN t.x = 'foo' THEN expensive_aggfunc1(srf1(t.y,t.z))
WHEN t.x = 'bar' THEN expensive_aggfunc2(srf2(t.y,t.z))
WHEN t.x = 'baz' THEN expensive_aggfunc3(srf3(t.y,t.z))
....
WHEN t.x = 'zzz' THEN expensive_aggfuncN(srfN(t.y,t.z))
ELSE 5
END
FROM mytable t;

Than to try to write it any other way.

I'm not advocating anything here, even though it may sound that way to you.
I'm just thinking this thing through, given that you may be committing a removal
of functionality that we want back at some later time.

Out of curiosity, how would you rewrite what I have above such that the
aggregate function is not inside the case statement, and the expensive_aggfuncs
are only called for those (t.y,t.z) that are actually appropriate?

Mark Dilger

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Dilger (#13)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

Mark Dilger <hornschnorter@gmail.com> writes:

On Jun 4, 2017, at 2:19 PM, Andres Freund <andres@anarazel.de> wrote:
Seems very unlikely that we'd ever want to do that. The right way to do
this is to simply move the SRF into the from list. Having the executor
support arbitrary sources of tuples would just complicate and slow down
already complicated and slow code...

In my example, the aggregate function is taking a column from the table as
an argument, so the output of the aggregate function needs to be computed per row,
not just once. And if the function is expensive, or has side-effects, you might
only want it to execute for those rows where the CASE statement is true, rather
than for all of them. You may get that same behavior using lateral or some such,
I'm uncertain, but in a complicated CASE statement, it be more straightforward
to write something like:

SELECT
CASE
WHEN t.x = 'foo' THEN expensive_aggfunc1(srf1(t.y,t.z))
WHEN t.x = 'bar' THEN expensive_aggfunc2(srf2(t.y,t.z))
WHEN t.x = 'baz' THEN expensive_aggfunc3(srf3(t.y,t.z))
....
WHEN t.x = 'zzz' THEN expensive_aggfuncN(srfN(t.y,t.z))
ELSE 5
END
FROM mytable t;

I think the correct way to do that already exists, namely to use a
correlated sub-select to wrap each SRF+aggregate:

...
WHEN t.x = 'foo' THEN (SELECT expensive_aggfunc1(s) FROM srf1(t.y,t.z) s)
...

I don't really feel a need to invent some other notation for that.

After chewing on this for awhile, I'm starting to come to the conclusion
that we'd be best off to throw an error for SRF-inside-CASE (or
COALESCE). Mark is correct that the simplest case of

SELECT x, CASE WHEN y THEN generate_series(1,z) ELSE 5 END
FROM table_with_columns_x_and_y_and_z;

behaves just intuitively enough that people might be using it. The new
implementation method cannot reasonably duplicate the old semantics for
that, which means that if we let it stand as-is we will be silently
breaking queries, even if we fix up some of the weirder corner cases like
what happens when the CASE can be const-simplified. So I think we'd be
better off to make this throw an error, and force any affected users to
rewrite in a way that will work in both v10 and older releases.

As to *how* to throw an error, I think it should be possible to teach
parse analysis to detect such cases, with something like the
ParseExprKind mechanism that could be checked to see if we're inside
a subexpression that restricts what's allowed. There are some other
checks like no-nested-aggregates that perhaps could be folded in as
well. Checking at parse analysis ought to be sufficient because
rule rewriting could not introduce such a case where it wasn't before,
and planner subquery flattening won't introduce one either because we
don't flatten subqueries with SRFs in their tlists.

If people are on board with throwing an error, I'll go see about
writing a patch.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#14)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

On Wed, Jun 7, 2017 at 11:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

If people are on board with throwing an error, I'll go see about
writing a patch.

+1 from me.

David J.​

#16Regina Obe
lr@pcorp.us
In reply to: Tom Lane (#14)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

After chewing on this for awhile, I'm starting to come to the conclusion

that we'd be best off to throw an error for SRF-inside-CASE (or COALESCE).
Mark is correct that the simplest case of

SELECT x, CASE WHEN y THEN generate_series(1,z) ELSE 5 END
FROM table_with_columns_x_and_y_and_z;

behaves just intuitively enough that people might be using it. The new

implementation method cannot reasonably duplicate the old semantics for
that, which means that if we let it stand as-is we will be

silently breaking queries, even if we fix up some of the weirder corner

cases like what happens when the CASE can be const-simplified. So I think
we'd be better off to make this throw an error, and force any

affected users to rewrite in a way that will work in both v10 and older

releases.

As to *how* to throw an error, I think it should be possible to teach

parse analysis to detect such cases, with something like the ParseExprKind
mechanism that could be checked to see if we're inside a

subexpression that restricts what's allowed. There are some other checks

like no-nested-aggregates that perhaps could be folded in as well. Checking
at parse analysis ought to be sufficient because

rule rewriting could not introduce such a case where it wasn't before, and

planner subquery flattening won't introduce one either because we don't
flatten subqueries with SRFs in their tlists.

If people are on board with throwing an error, I'll go see about writing a

patch.

regards, tom lane

+1

I'm not a fan of either solution, but I think what Tom proposes of throwing
an error sounds like least invasive and confusing.

I'd much prefer an error thrown than silent behavior change. Given that we
ran into this in 3 places in PostGIS code, I'm not convinced the issue is
all that rare.

Make sure to point out the breaking change in the release notes though and
syntax to remedy it.

Thanks,
Regina

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Regina Obe (#16)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

"Regina Obe" <lr@pcorp.us> writes:

I'm not a fan of either solution, but I think what Tom proposes of throwing
an error sounds like least invasive and confusing.

I'd much prefer an error thrown than silent behavior change. Given that we
ran into this in 3 places in PostGIS code, I'm not convinced the issue is
all that rare.

Make sure to point out the breaking change in the release notes though and
syntax to remedy it.

As far as that goes, the best fix I could think of after a few minutes is
to integrate your conditional logic into a custom set-returning function.
For example,

select x, case when y > 0 then generate_series(1, z) else 5 end from tt;

could become

create function mysrf(cond bool, start int, fin int, els int)
returns setof int as $$
begin
if cond then
return query select generate_series(start, fin);
else
return query select els;
end if;
end$$ language plpgsql;

select x, mysrf(y > 0, 1, z, 5) from tt;

(adjust the amount of parameterization to taste, of course)

Now, the fact that a fairly mechanical conversion like this is possible
suggests that we *could* solve the problem if we had to, at least for
simple cases like this one. But it'd be a lot of work, not least because
we'd presumably not want core-defined syntax to depend on an extension
like plpgsql --- and I don't see a way to do this with straight SQL
functions. So my feeling is that we should not expend that effort.
If it turns out that a lot more people are affected than I currently
think will be the case, maybe we'll have to revisit that choice.

But this line of thinking does strengthen my feeling that throwing an
error is the right thing to do for the moment. If we allow v10 to accept
such cases but do something different from what we used to, that will
greatly complicate any future attempt to try to restore the old behavior.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#18Regina Obe
lr@pcorp.us
In reply to: Tom Lane (#17)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

But this line of thinking does strengthen my feeling that throwing an

error is the right thing to do for the moment. If we allow v10 to accept
such cases but do something different from what we used to, that

will greatly complicate any future attempt to try to restore the old

behavior.

regards, tom lane

Agreed. The other side benefit of throwing an error instead of just doing
something different is you'll find out how rampant the old behavior is :).

People are more likely to know to complain when their apps break than they
are if it just silently starts doing something different.

My main concern in these cases is the short-circuiting not happening.
Because in these cases, the code goes into areas that it shouldn't which is
likely to mess up some logic in hard to troubleshoot ways.
I think erroring out is the best compromise.

Thanks,
Regina

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#19Andres Freund
andres@anarazel.de
In reply to: Regina Obe (#18)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

On 2017-06-08 11:57:49 -0400, Regina Obe wrote:

My main concern in these cases is the short-circuiting not happening.

Note there's also no short-circuiting e.g. for aggregates inside case
either.

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#20Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#19)
Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

"'Andres Freund'" <andres@anarazel.de> writes:

On 2017-06-08 11:57:49 -0400, Regina Obe wrote:

My main concern in these cases is the short-circuiting not happening.

Note there's also no short-circuiting e.g. for aggregates inside case
either.

Well, depends. If const-folding manages to get rid of the aggregate
call altogether, it won't be computed.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#14)
#22Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#21)
#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#22)
#24Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#23)
#25Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#24)
#26Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#25)
#27Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#24)
#28Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#27)
#29Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#28)