Thread: Should we document IS [NOT] OF?

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Over in news [1] Josh Drake and Eric Ridge discovered the undocumented
> feature "IS [NOT] OF"; introduced seemingly as an "oh-by-the-way" in 2002
> via commit eb121ba2cfe [2].

> Is there any reason not to document this back to 9.5,

As far as I can tell from reading the SQL spec, this has nothing much in
common with the SQL feature of that name except for the syntax.  The SQL
feature seems to be a *run time* test on subtype inclusion, not something
that can be answered in parse analysis.  Even if I'm getting that wrong,
it's clear that the spec intends IS OF to return true for subtype
relationships, not only exact type equality which is the only thing
transformAExprOf considers.

So my vote would be to rip it out, not document it.  Somebody can try
again in future, perhaps.  But if we document it we're just locking
ourselves into a SQL incompatibility.

            regards, tom lane

I wrote:
> So my vote would be to rip it out, not document it.  Somebody can try
> again in future, perhaps.  But if we document it we're just locking
> ourselves into a SQL incompatibility.

Apparently, somebody already had that thought.  See func.sgml
lines 765-782, which were commented out by 8272fc3f7.

            regards, tom lane

I wrote:
> So my vote would be to rip it out, not document it.  Somebody can try
> again in future, perhaps.  But if we document it we're just locking
> ourselves into a SQL incompatibility.

Apparently, somebody already had that thought.  See func.sgml
lines 765-782, which were commented out by 8272fc3f7.

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Is there a feature code? I skimmed the standard and non-standard tables in
> our appendix and couldn’t find this in either.

a19d9d3c4 seems to have thought it was S151.

            regards, tom lane

On 11/19/20 2:03 AM, Tom Lane wrote:
> "David G. Johnston" <david.g.johnston@gmail.com> writes:
>> Is there a feature code? I skimmed the standard and non-standard tables in
>> our appendix and couldn’t find this in either.
>
> a19d9d3c4 seems to have thought it was S151.

Here is a link to previous list discussions:

https://www.postgresql.org/message-id/45DA44F3.3010401%40joeconway.com

HTH,

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Joe Conway <mail@joeconway.com> writes:
> Here is a link to previous list discussions:
> https://www.postgresql.org/message-id/45DA44F3.3010401%40joeconway.com

Ah, thanks, I was intending to go do some searching for that.

So at this point this has been re-debated at least three times.
I think it's time to put it out of its misery.  You can get
equivalent behavior using something like
   pg_typeof(foo) in ('int'::regtype, 'float8'::regtype)
so we've got the "another way to do it" covered.  And I'm
still convinced that the existing implementation is not
anywhere near per-spec.  The issue about NULL is somewhat
minor perhaps, but the real problem is that I think the
spec intends "foo is of (sometype)" to return true if foo
is of any subtype of sometype.  We don't have subtypes in
the way SQL intends, but we do have inheritance children
which are more or less the same idea.  The closest you can
get right now is to do something like

   select * from parent_table t where t.tableoid = 'child1'::regclass

but this will fail to match grandchilden of child1.
I think a minimum expectation is that you could do
something like "where (t.*) is of (child1)", but
our existing code is nowhere near making that work.

Let's just rip it out and be done.  If anyone is ever
motivated to make it work per spec, they can resurrect
whatever seems useful from the git history.

            regards, tom lane

On 11/19/20 11:06 AM, Tom Lane wrote:
> Let's just rip it out and be done.  If anyone is ever
> motivated to make it work per spec, they can resurrect
> whatever seems useful from the git history.

+1

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

On Thu, Nov 19, 2020 at 11:15:33AM -0500, Joe Conway wrote:
> On 11/19/20 11:06 AM, Tom Lane wrote:
> > Let's just rip it out and be done.  If anyone is ever
> > motivated to make it work per spec, they can resurrect
> > whatever seems useful from the git history.
> 
> +1

+1

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

Bruce Momjian <bruce@momjian.us> writes:
> On Thu, Nov 19, 2020 at 11:15:33AM -0500, Joe Conway wrote:
>> On 11/19/20 11:06 AM, Tom Lane wrote:
>>> Let's just rip it out and be done.  If anyone is ever
>>> motivated to make it work per spec, they can resurrect
>>> whatever seems useful from the git history.

>> +1

> +1

Here's a proposed patch for that.  I was amused to discover that we have
a couple of regression test cases making use of IS OF.  However, I think
using pg_typeof() is actually better for those tests anyway, since
printing the regtype result is clearer, and easier to debug if the test
ever goes wrong.

            regards, tom lane

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index df2c1c6f05..7d06b979eb 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -762,25 +762,6 @@ repeat('Pg', 4) <returnvalue>PgPgPgPg</returnvalue>
     expression must be of Boolean type.
    </para>

-<!-- IS OF does not conform to the ISO SQL behavior, so it is undocumented here
-   <para>
-    <indexterm>
-     <primary>IS OF</primary>
-    </indexterm>
-    <indexterm>
-     <primary>IS NOT OF</primary>
-    </indexterm>
-    It is possible to check the data type of an expression using the
-    predicates
-<synopsis>
-<replaceable>expression</replaceable> IS OF (typename, ...)
-<replaceable>expression</replaceable> IS NOT OF (typename, ...)
-</synopsis>
-    They return a boolean value based on whether the expression's data
-    type is one of the listed data types.
-   </para>
--->
-
    <para>
     Some comparison-related functions are also available, as shown in <xref
     linkend="functions-comparison-func-table"/>.
diff --git a/src/backend/catalog/sql_features.txt b/src/backend/catalog/sql_features.txt
index b6e58e8493..caa971c435 100644
--- a/src/backend/catalog/sql_features.txt
+++ b/src/backend/catalog/sql_features.txt
@@ -373,7 +373,7 @@ S096    Optional array bounds            YES
 S097    Array element assignment            NO
 S098    ARRAY_AGG            YES
 S111    ONLY in query expressions            YES
-S151    Type predicate            NO
+S151    Type predicate            NO    see pg_typeof()
 S161    Subtype treatment            NO
 S162    Subtype treatment for references            NO
 S201    SQL-invoked routines on arrays            YES
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 4504b1503b..f26498cea2 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -3213,10 +3213,6 @@ _outAExpr(StringInfo str, const A_Expr *node)
             appendStringInfoString(str, " NULLIF ");
             WRITE_NODE_FIELD(name);
             break;
-        case AEXPR_OF:
-            appendStringInfoString(str, " OF ");
-            WRITE_NODE_FIELD(name);
-            break;
         case AEXPR_IN:
             appendStringInfoString(str, " IN ");
             WRITE_NODE_FIELD(name);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 2cb377d034..efc9c99754 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -13238,14 +13238,6 @@ a_expr:        c_expr                                    { $$ = $1; }
                 {
                     $$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_DISTINCT, "=", $1, $6, @2);
                 }
-            | a_expr IS OF '(' type_list ')'            %prec IS
-                {
-                    $$ = (Node *) makeSimpleA_Expr(AEXPR_OF, "=", $1, (Node *) $5, @2);
-                }
-            | a_expr IS NOT OF '(' type_list ')'        %prec IS
-                {
-                    $$ = (Node *) makeSimpleA_Expr(AEXPR_OF, "<>", $1, (Node *) $6, @2);
-                }
             | a_expr BETWEEN opt_asymmetric b_expr AND a_expr        %prec BETWEEN
                 {
                     $$ = (Node *) makeSimpleA_Expr(AEXPR_BETWEEN,
@@ -13464,14 +13456,6 @@ b_expr:        c_expr
                 {
                     $$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_DISTINCT, "=", $1, $6, @2);
                 }
-            | b_expr IS OF '(' type_list ')'        %prec IS
-                {
-                    $$ = (Node *) makeSimpleA_Expr(AEXPR_OF, "=", $1, (Node *) $5, @2);
-                }
-            | b_expr IS NOT OF '(' type_list ')'    %prec IS
-                {
-                    $$ = (Node *) makeSimpleA_Expr(AEXPR_OF, "<>", $1, (Node *) $6, @2);
-                }
             | b_expr IS DOCUMENT_P                    %prec IS
                 {
                     $$ = makeXmlExpr(IS_DOCUMENT, NULL, NIL,
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index f5165863d7..36002f059d 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -94,7 +94,6 @@ static Node *transformAExprOpAny(ParseState *pstate, A_Expr *a);
 static Node *transformAExprOpAll(ParseState *pstate, A_Expr *a);
 static Node *transformAExprDistinct(ParseState *pstate, A_Expr *a);
 static Node *transformAExprNullIf(ParseState *pstate, A_Expr *a);
-static Node *transformAExprOf(ParseState *pstate, A_Expr *a);
 static Node *transformAExprIn(ParseState *pstate, A_Expr *a);
 static Node *transformAExprBetween(ParseState *pstate, A_Expr *a);
 static Node *transformBoolExpr(ParseState *pstate, BoolExpr *a);
@@ -228,9 +227,6 @@ transformExprRecurse(ParseState *pstate, Node *expr)
                     case AEXPR_NULLIF:
                         result = transformAExprNullIf(pstate, a);
                         break;
-                    case AEXPR_OF:
-                        result = transformAExprOf(pstate, a);
-                        break;
                     case AEXPR_IN:
                         result = transformAExprIn(pstate, a);
                         break;
@@ -1168,51 +1164,6 @@ transformAExprNullIf(ParseState *pstate, A_Expr *a)
     return (Node *) result;
 }

-/*
- * Checking an expression for match to a list of type names. Will result
- * in a boolean constant node.
- */
-static Node *
-transformAExprOf(ParseState *pstate, A_Expr *a)
-{
-    Node       *lexpr = a->lexpr;
-    Const       *result;
-    ListCell   *telem;
-    Oid            ltype,
-                rtype;
-    bool        matched = false;
-
-    if (operator_precedence_warning)
-        emit_precedence_warnings(pstate, PREC_GROUP_POSTFIX_IS, "IS",
-                                 lexpr, NULL,
-                                 a->location);
-
-    lexpr = transformExprRecurse(pstate, lexpr);
-
-    ltype = exprType(lexpr);
-    foreach(telem, (List *) a->rexpr)
-    {
-        rtype = typenameTypeId(pstate, lfirst(telem));
-        matched = (rtype == ltype);
-        if (matched)
-            break;
-    }
-
-    /*
-     * We have two forms: equals or not equals. Flip the sense of the result
-     * for not equals.
-     */
-    if (strcmp(strVal(linitial(a->name)), "<>") == 0)
-        matched = (!matched);
-
-    result = (Const *) makeBoolConst(matched, false);
-
-    /* Make the result have the original input's parse location */
-    result->location = exprLocation((Node *) a);
-
-    return (Node *) result;
-}
-
 static Node *
 transformAExprIn(ParseState *pstate, A_Expr *a)
 {
@@ -3257,11 +3208,6 @@ operator_precedence_group(Node *node, const char **nodename)
             *nodename = "IS";
             group = PREC_GROUP_INFIX_IS;
         }
-        else if (aexpr->kind == AEXPR_OF)
-        {
-            *nodename = "IS";
-            group = PREC_GROUP_POSTFIX_IS;
-        }
         else if (aexpr->kind == AEXPR_IN)
         {
             *nodename = "IN";
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index a2dcdef3fa..d1f9ef29ca 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -258,7 +258,6 @@ typedef enum A_Expr_Kind
     AEXPR_DISTINCT,                /* IS DISTINCT FROM - name must be "=" */
     AEXPR_NOT_DISTINCT,            /* IS NOT DISTINCT FROM - name must be "=" */
     AEXPR_NULLIF,                /* NULLIF - name must be "=" */
-    AEXPR_OF,                    /* IS [NOT] OF - name must be "=" or "<>" */
     AEXPR_IN,                    /* [NOT] IN - name must be "=" or "<>" */
     AEXPR_LIKE,                    /* [NOT] LIKE - name must be "~~" or "!~~" */
     AEXPR_ILIKE,                /* [NOT] ILIKE - name must be "~~*" or "!~~*" */
diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index 5abfeb6773..e0f870e9d7 100644
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -1149,10 +1149,10 @@ SELECT ARRAY[1,2,3]::text[]::int[]::float8[] AS "{1,2,3}";
  {1,2,3}
 (1 row)

-SELECT ARRAY[1,2,3]::text[]::int[]::float8[] is of (float8[]) as "TRUE";
- TRUE
-------
- t
+SELECT pg_typeof(ARRAY[1,2,3]::text[]::int[]::float8[]) AS "double precision[]";
+ double precision[]
+--------------------
+ double precision[]
 (1 row)

 SELECT ARRAY[['a','bc'],['def','hijk']]::text[]::varchar[] AS "{{a,bc},{def,hijk}}";
@@ -1161,10 +1161,10 @@ SELECT ARRAY[['a','bc'],['def','hijk']]::text[]::varchar[] AS "{{a,bc},{def,hijk
  {{a,bc},{def,hijk}}
 (1 row)

-SELECT ARRAY[['a','bc'],['def','hijk']]::text[]::varchar[] is of (varchar[]) as "TRUE";
- TRUE
-------
- t
+SELECT pg_typeof(ARRAY[['a','bc'],['def','hijk']]::text[]::varchar[]) AS "character varying[]";
+ character varying[]
+---------------------
+ character varying[]
 (1 row)

 SELECT CAST(ARRAY[[[[[['a','bb','ccc']]]]]] as text[]) as "{{{{{{a,bb,ccc}}}}}}";
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index 2a033a6e11..411d5c003e 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -70,22 +70,16 @@ from basictest;
 (3 rows)

 -- check that union/case/coalesce type resolution handles domains properly
-select coalesce(4::domainint4, 7) is of (int4) as t;
- t
----
- t
-(1 row)
-
-select coalesce(4::domainint4, 7) is of (domainint4) as f;
- f
----
- f
+select pg_typeof(coalesce(4::domainint4, 7));
+ pg_typeof
+-----------
+ integer
 (1 row)

-select coalesce(4::domainint4, 7::domainint4) is of (domainint4) as t;
- t
----
- t
+select pg_typeof(coalesce(4::domainint4, 7::domainint4));
+ pg_typeof
+------------
+ domainint4
 (1 row)

 drop table basictest;
diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out
index 96835a517e..9429e9fd28 100644
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -142,10 +142,10 @@ SELECT * FROM t LIMIT 10;

 -- Test behavior with an unknown-type literal in the WITH
 WITH q AS (SELECT 'foo' AS x)
-SELECT x, x IS OF (text) AS is_text FROM q;
-  x  | is_text
------+---------
- foo | t
+SELECT x, pg_typeof(x) FROM q;
+  x  | pg_typeof
+-----+-----------
+ foo | text
 (1 row)

 WITH RECURSIVE t(n) AS (
@@ -153,15 +153,15 @@ WITH RECURSIVE t(n) AS (
 UNION ALL
     SELECT n || ' bar' FROM t WHERE length(n) < 20
 )
-SELECT n, n IS OF (text) AS is_text FROM t;
-            n            | is_text
--------------------------+---------
- foo                     | t
- foo bar                 | t
- foo bar bar             | t
- foo bar bar bar         | t
- foo bar bar bar bar     | t
- foo bar bar bar bar bar | t
+SELECT n, pg_typeof(n) FROM t;
+            n            | pg_typeof
+-------------------------+-----------
+ foo                     | text
+ foo bar                 | text
+ foo bar bar             | text
+ foo bar bar bar         | text
+ foo bar bar bar bar     | text
+ foo bar bar bar bar bar | text
 (6 rows)

 -- In a perfect world, this would work and resolve the literal as int ...
@@ -171,7 +171,7 @@ WITH RECURSIVE t(n) AS (
 UNION ALL
     SELECT n+1 FROM t WHERE n < 10
 )
-SELECT n, n IS OF (int) AS is_int FROM t;
+SELECT n, pg_typeof(n) FROM t;
 ERROR:  operator does not exist: text + integer
 LINE 4:     SELECT n+1 FROM t WHERE n < 10
                     ^
diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index 906fd712b7..199049b2a2 100644
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -343,9 +343,9 @@ SELECT * FROM array_op_test WHERE t <@ '{}' ORDER BY seqno;

 -- array casts
 SELECT ARRAY[1,2,3]::text[]::int[]::float8[] AS "{1,2,3}";
-SELECT ARRAY[1,2,3]::text[]::int[]::float8[] is of (float8[]) as "TRUE";
+SELECT pg_typeof(ARRAY[1,2,3]::text[]::int[]::float8[]) AS "double precision[]";
 SELECT ARRAY[['a','bc'],['def','hijk']]::text[]::varchar[] AS "{{a,bc},{def,hijk}}";
-SELECT ARRAY[['a','bc'],['def','hijk']]::text[]::varchar[] is of (varchar[]) as "TRUE";
+SELECT pg_typeof(ARRAY[['a','bc'],['def','hijk']]::text[]::varchar[]) AS "character varying[]";
 SELECT CAST(ARRAY[[[[[['a','bb','ccc']]]]]] as text[]) as "{{{{{{a,bb,ccc}}}}}}";
 SELECT NULL::text[]::int[] AS "NULL";

diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index 1291d550d6..549c0b5adf 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -59,9 +59,8 @@ select testtext || testvarchar as concat, testnumeric + 42 as sum
 from basictest;

 -- check that union/case/coalesce type resolution handles domains properly
-select coalesce(4::domainint4, 7) is of (int4) as t;
-select coalesce(4::domainint4, 7) is of (domainint4) as f;
-select coalesce(4::domainint4, 7::domainint4) is of (domainint4) as t;
+select pg_typeof(coalesce(4::domainint4, 7));
+select pg_typeof(coalesce(4::domainint4, 7::domainint4));

 drop table basictest;
 drop domain domainvarchar restrict;
diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql
index b1b79eb172..ad976de531 100644
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -77,14 +77,14 @@ SELECT * FROM t LIMIT 10;

 -- Test behavior with an unknown-type literal in the WITH
 WITH q AS (SELECT 'foo' AS x)
-SELECT x, x IS OF (text) AS is_text FROM q;
+SELECT x, pg_typeof(x) FROM q;

 WITH RECURSIVE t(n) AS (
     SELECT 'foo'
 UNION ALL
     SELECT n || ' bar' FROM t WHERE length(n) < 20
 )
-SELECT n, n IS OF (text) AS is_text FROM t;
+SELECT n, pg_typeof(n) FROM t;

 -- In a perfect world, this would work and resolve the literal as int ...
 -- but for now, we have to be content with resolving to text too soon.
@@ -93,7 +93,7 @@ WITH RECURSIVE t(n) AS (
 UNION ALL
     SELECT n+1 FROM t WHERE n < 10
 )
-SELECT n, n IS OF (int) AS is_int FROM t;
+SELECT n, pg_typeof(n) FROM t;

 --
 -- Some examples with a tree

On 11/19/20 12:08 PM, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
>> On Thu, Nov 19, 2020 at 11:15:33AM -0500, Joe Conway wrote:
>>> On 11/19/20 11:06 AM, Tom Lane wrote:
>>>> Let's just rip it out and be done.  If anyone is ever
>>>> motivated to make it work per spec, they can resurrect
>>>> whatever seems useful from the git history.
>
>>> +1
>
>> +1
>
> Here's a proposed patch for that.  I was amused to discover that we have
> a couple of regression test cases making use of IS OF.


I didn't check but those might be my fault ;-)


> However, I think using pg_typeof() is actually better for those tests anyway, since
> printing the regtype result is clearer, and easier to debug if the test
> ever goes wrong.

Looks good to me.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Joe Conway <mail@joeconway.com> writes:
> On 11/19/20 12:08 PM, Tom Lane wrote:
>> Here's a proposed patch for that.  I was amused to discover that we have
>> a couple of regression test cases making use of IS OF.

> I didn't check but those might be my fault ;-)

I suspect at least one of them is mine ;-).  But I didn't check.

> Looks good to me.

Thanks for looking!

            regards, tom lane

After digging a bit more I noticed that we'd discussed removing
IS OF in the 2007 thread, but forebore because there wasn't an easy
replacement.  pg_typeof() was added a year later (b8fab2411), so we
could have done this at any point since then.

Pushed.

            regards, tom lane