Thread: BUG #11617: issue with dump/restore involving view with hstore data type embedded in where condition

The following bug has been logged on the website:

Bug reference:      11617
Logged by:          Normand Desautels
Email address:      support@maerix.com
PostgreSQL version: 9.3.4
Operating system:   Ubuntu 12.04 LTS
Description:

------------------------
Specs:

Distributor ID:    Ubuntu
Description:    Ubuntu 12.04 LTS
Release:    12.04
Codename:    precise

PG Version:  PostgreSQL 9.3.4 on x86_64-unknown-linux-gnu, compiled by gcc
(Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3, 64-bi
-------------------------

To whom it may concern,

in restoring a dump followed by a restore, the restore partially work's but
does not restore a specific view with a hstore component

The error message is:

pg_restore: [archiver (db)] could not execute query: ERROR:  operator does
not exist: public.hstore = public.hstore
LINE 30: ...me <> 'fk_formation_id'::text)) AND (h1.old_value IS DISTINC...
                                                              ^
HINT:  No operator matches the given name and argument type(s). You might
need to add explicit type casts.


The condition in the WHERE clause looks like  this:

"WHERE (h1.old_value IS DISTINCT FROM h1.new_value)"

Where the columns old_value and new_value are of the HSTORE datatype, and
are optionnal.

That is why we called upon "IS DISTINCT FROM " to manage possible NULL
values.

AS a workaround, we used instead COALESCE with a fake hstore value for NULL
cases

Something along the line of:

where COALESCE( h1.old_value,'"e"=>"1", "f"=>"2"'::hstore) <>
COALESCE(h2.new_value,'"e"=>"1", "f"=>"2"'::hstore);

And it resolved the issue when restoring. Everything goes through normally.

Now, either it is a bug similar to the NULLIF issue (see attached
http://stackoverflow.com/q/23599926/398670)
This bug was listed on the 12th of May by Craig Ringer.

Or the use of "IS DISTINCT FROM" clause is irreconcilable with hstore
datatypes.
If that is the case, we will take note of this.

Can you clarify this.

Thanks for your looking it.

Regards

On 10/09/2014 11:00 PM, support@maerix.com wrote:
> in restoring a dump followed by a restore, the restore partially work's but
> does not restore a specific view with a hstore component
>
> The error message is:
>
> pg_restore: [archiver (db)] could not execute query: ERROR:  operator does
> not exist: public.hstore = public.hstore
> LINE 30: ...me <> 'fk_formation_id'::text)) AND (h1.old_value IS DISTINC...
>                                                                ^
> HINT:  No operator matches the given name and argument type(s). You might
> need to add explicit type casts.
>
>
> The condition in the WHERE clause looks like  this:
>
> "WHERE (h1.old_value IS DISTINCT FROM h1.new_value)"
>
> Where the columns old_value and new_value are of the HSTORE datatype, and
> are optionnal.
>
> That is why we called upon "IS DISTINCT FROM " to manage possible NULL
> values.
>
> AS a workaround, we used instead COALESCE with a fake hstore value for NULL
> cases
>
> Something along the line of:
>
> where COALESCE( h1.old_value,'"e"=>"1", "f"=>"2"'::hstore) <>
> COALESCE(h2.new_value,'"e"=>"1", "f"=>"2"'::hstore);
>
> And it resolved the issue when restoring. Everything goes through normally.
>
> Now, either it is a bug similar to the NULLIF issue (see attached
> http://stackoverflow.com/q/23599926/398670)
> This bug was listed on the 12th of May by Craig Ringer.
>
> Or the use of "IS DISTINCT FROM" clause is irreconcilable with hstore
> datatypes.
> If that is the case, we will take note of this.

Yeah, this is essentially the same bug as with NULLIF.

In the catalogs, we store the OID of the equality operator used in a
NULLIF or IS DISTINCT FROM expression. But when you try to deparse that
back to an SQL statement, it's impossible to construct an equivalent SQL
statement that would refer to the same operator, when that operator is
not in search_path. In essence, it's not possible to schema-qualify the
equality operator used.

There are a whole bunch of expressions that have the same problem:
a IS DISTINCT FROM b
a IS NOT DISTINCT FROM b
a IN (...)
a NOT IN (...)
CASE a WHEN b ... ELSE d END
NULLIF(a, b)

I don't think this can be solved without some additional syntax, for
specifying the equality operator explicitly. I propose that we add an
optional USING <operator> after the problematic expressions:

a IS DISTINCT FROM b USING myschema.=
NULLIF(a, b) USING myschema.=
...

I gave that a quick try, but got a shift/reduce conflict. I don't have
the time to dig deeper right now, but it might be that that particular
syntax might not work out. But something like that.

(NULLIF, IS DISTINCT FROM and CASE are specified by the SQL standard, so
there's a small risk that the standard committee might extend the syntax
in a way that conflicts with this. But it seems highly unlikely.)

- Heikki

Heikki Linnakangas <hlinnakangas@vmware.com> writes:
> In the catalogs, we store the OID of the equality operator used in a
> NULLIF or IS DISTINCT FROM expression. But when you try to deparse that
> back to an SQL statement, it's impossible to construct an equivalent SQL
> statement that would refer to the same operator, when that operator is
> not in search_path. In essence, it's not possible to schema-qualify the
> equality operator used.

Right.

> I don't think this can be solved without some additional syntax, for
> specifying the equality operator explicitly. I propose that we add an
> optional USING <operator> after the problematic expressions:

> a IS DISTINCT FROM b USING myschema.=
> NULLIF(a, b) USING myschema.=
> ...

Meh.  When would this be used?  I don't think we'd want ruleutils to
deparse this way all the time.  Also, in the case of IS DISTINCT FROM
on composite values, it's not clear that one operator name is enough.

I wonder whether we couldn't fix this better by insisting that these
operations depend on default btree opclasses instead of looking up
"=" by name.  Upthread I whined that this wouldn't work for comparisons
of nonidentical datatypes, but could we insist for cross-type cases that
both types have default btree opclasses belonging to the same opfamily?

It's possible that such a redefinition would lead to rejecting some
cases that work today, but I think they'd be strange corner cases.
A quick look through pg_operator suggests that there are no built-in
operators named "=" that link input types that aren't in the same
opfamily.

The big-picture situation is that right now, we have a weird mixture of
cases where we do equality or sorting on the basis of operators looked up
with an implicitly-assumed operator name, and cases where we do it based
on finding a suitable member of a default btree opclass.  The second way
is on far more solid ground theoretically IMO.  The only defense of the
first way is that the SQL spec frequently says in so many words "this
syntax is equivalent to A = B", and if you take that in a very literal
fashion then looking up an operator named "=" is the thing to do.  But
I think a reasonable case could be made that they mean "A equals B" in
some more abstract sense than "what is the operator named?".

            regards, tom lane

I wrote:
> I wonder whether we couldn't fix this better by insisting that these
> operations depend on default btree opclasses instead of looking up
> "=" by name.  Upthread I whined that this wouldn't work for comparisons
> of nonidentical datatypes, but could we insist for cross-type cases that
> both types have default btree opclasses belonging to the same opfamily?

I thought a bit more about this, and that idea isn't going to work, at
least not by itself.  Right now you can do, eg, "integer IS DISTINCT
FROM numeric", and it works, but the way it works is that the integer
is promoted to numeric and we use the "numeric = numeric" operator.
int and numeric do not have an opfamily in common, so this case would
fail with the rule above.

A reasonable way to handle that sort of case is to do select_common_type
(ie, UNION-like type promotion) and then insist that the common type have
a default btree opclass.

However, the common-opfamily approach seems like a better idea when it
works, since it would avoid a runtime type coercion in cases where there
is a suitable cross-type comparison operator.  So what I now suggest is:

1. If the two inputs are of types that have default btree opclasses in
the same opfamily, and that opfamily has an equality operator that
accepts these input types, use that operator.

2. Otherwise, do select_common_type(), and see if the common type has
a default btree opclass.  If so, use that opclass's equality operator
after coercing both inputs to the common type.

3. If neither of those work, fail.

            regards, tom lane

On Sat, Oct 25, 2014 at 09:28:27PM -0400, Tom Lane wrote:
> I wrote:
> > I wonder whether we couldn't fix this better by insisting that these
> > operations depend on default btree opclasses instead of looking up
> > "=" by name.  Upthread I whined that this wouldn't work for comparisons
> > of nonidentical datatypes, but could we insist for cross-type cases that
> > both types have default btree opclasses belonging to the same opfamily?
>
> I thought a bit more about this, and that idea isn't going to work, at
> least not by itself.  Right now you can do, eg, "integer IS DISTINCT
> FROM numeric", and it works, but the way it works is that the integer
> is promoted to numeric and we use the "numeric = numeric" operator.
> int and numeric do not have an opfamily in common, so this case would
> fail with the rule above.

Is there something precluding implementation of an all-numeric-types opfamily
that contains the existing default btree opclass operators?  That wouldn't
solve every example like this, but it would help here among other places.

> A reasonable way to handle that sort of case is to do select_common_type
> (ie, UNION-like type promotion) and then insist that the common type have
> a default btree opclass.
>
> However, the common-opfamily approach seems like a better idea when it
> works, since it would avoid a runtime type coercion in cases where there
> is a suitable cross-type comparison operator.  So what I now suggest is:
>
> 1. If the two inputs are of types that have default btree opclasses in
> the same opfamily, and that opfamily has an equality operator that
> accepts these input types, use that operator.
>
> 2. Otherwise, do select_common_type(), and see if the common type has
> a default btree opclass.  If so, use that opclass's equality operator
> after coercing both inputs to the common type.
>
> 3. If neither of those work, fail.

I'm loathe to introduce another operator selection method.  We have the
func_select_candidates() method and the opfamily method.  This third method
shares some features with each of the existing two, and it makes novel use of
select_common_type().  To break compatibility like this, we had better be
confident that the new algorithm is a great one.  I'm confident that the
current specification is a bad one, but I'm not confident that bringing
select_common_type() into the mix is the right replacement.

On Fri, Oct 24, 2014 at 06:36:47PM -0400, Tom Lane wrote:
> Heikki Linnakangas <hlinnakangas@vmware.com> writes:
> > I don't think this can be solved without some additional syntax, for
> > specifying the equality operator explicitly. I propose that we add an
> > optional USING <operator> after the problematic expressions:
>
> > a IS DISTINCT FROM b USING myschema.=
> > NULLIF(a, b) USING myschema.=
> > ...
>
> Meh.  When would this be used?  I don't think we'd want ruleutils to
> deparse this way all the time.  Also, in the case of IS DISTINCT FROM
> on composite values, it's not clear that one operator name is enough.

The similar situation with IN -> =ANY is tolerable, and I could be content
with ruleutils deparsing that way every time.  That's certainly the safe fix.

Thanks,
nm

Noah Misch <noah@leadboat.com> writes:
> On Sat, Oct 25, 2014 at 09:28:27PM -0400, Tom Lane wrote:
>> I thought a bit more about this, and that idea isn't going to work, at
>> least not by itself.  Right now you can do, eg, "integer IS DISTINCT
>> FROM numeric", and it works, but the way it works is that the integer
>> is promoted to numeric and we use the "numeric = numeric" operator.
>> int and numeric do not have an opfamily in common, so this case would
>> fail with the rule above.

> Is there something precluding implementation of an all-numeric-types opfamily
> that contains the existing default btree opclass operators?  That wouldn't
> solve every example like this, but it would help here among other places.

That solves only the first example I came up with, not the general
problem.  Another case of the same ilk is "int IS DISTINCT FROM float8",
which also works today, and which we *cannot* fix by merging the int and
float opclasses.  (Equality would not be transitive in such an opfamily.)
text vs bpchar comparison is another example, and I'm sure there are more.

>> Heikki Linnakangas <hlinnakangas@vmware.com> writes:
>>> I don't think this can be solved without some additional syntax, for
>>> specifying the equality operator explicitly. I propose that we add an
>>> optional USING <operator> after the problematic expressions:

> The similar situation with IN -> =ANY is tolerable, and I could be content
> with ruleutils deparsing that way every time.  That's certainly the safe fix.

That's completely not comparable, because =ANY is still valid/standard
SQL syntax.  I don't see how you can justify the idea that introducing
new, nonstandard syntax for a fundamental SQL operator is better than
introducing a new operator selection rule.

Keep in mind also that IS DISTINCT FROM is just the tip of the iceberg.
There's also the shorthand CASE syntax, and probably some other cases
that I'm not recalling in my caffeine-deprived state.

            regards, tom lane

On Wed, Jan 07, 2015 at 10:12:10AM -0500, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > Is there something precluding implementation of an all-numeric-types opfamily
> > that contains the existing default btree opclass operators?  That wouldn't
> > solve every example like this, but it would help here among other places.
>
> That solves only the first example I came up with, not the general
> problem.

Correct.

> >> Heikki Linnakangas <hlinnakangas@vmware.com> writes:
> >>> I don't think this can be solved without some additional syntax, for
> >>> specifying the equality operator explicitly. I propose that we add an
> >>> optional USING <operator> after the problematic expressions:
>
> > The similar situation with IN -> =ANY is tolerable, and I could be content
> > with ruleutils deparsing that way every time.  That's certainly the safe fix.
>
> That's completely not comparable, because =ANY is still valid/standard
> SQL syntax.  I don't see how you can justify the idea that introducing
> new, nonstandard syntax for a fundamental SQL operator is better than
> introducing a new operator selection rule.

I weight various priorities as follows; higher is better:

8 - backward compatibility for applications
7 - dump/restore cycle working
5 - query behavior predictable by non-hackers
2 - beauty of ruleutils.c output
1 - limiting use of nonstandard SQL in ruleutils.c output

Hence my conclusion above.  Also, I have more than once written an application
query that would have used IS DISTINCT but for the inability to reach a
specific operator; the proposed syntax is an independently-useful feature.

> Keep in mind also that IS DISTINCT FROM is just the tip of the iceberg.
> There's also the shorthand CASE syntax, and probably some other cases
> that I'm not recalling in my caffeine-deprived state.

The list:

IS [NOT] DISTINCT
[NOT] IN
JOIN USING, NATURAL JOIN
CASE <expr> WHEN
NULLIF