Thread: Re: [HACKERS] IS OF

Re: [HACKERS] IS OF

From
Joe Conway
Date:
Bruce Momjian wrote:
> Can someone suggest where to document IS OF, and either document it's
> non-standard behavior or supply patch?
>

Doc patch attached for IS OF. Please apply.

Thanks,

Joe


> ---------------------------------------------------------------------------
>
> Joe Conway wrote:
>
>>Gavin Sherry wrote:
>>
>>>8.14 <type predicate> to be exact.
>>>
>>
>>8.18 in SQL200x. I don't think the current implementation quite meets
>>the spec however:
>>
>>regression=# select f2 is null, f2 is of(int) from bar;
>>  ?column? | ?column?
>>----------+----------
>>  f        | t
>>  t        | t
>>(2 rows)
>>
>>If I read the spec correctly, the null value should return null, not 't'
>>in the above.
>>
>>General Rules
>>1) Let V be the result of evaluating the <row value predicand>.
>>2) Let ST be the set consisting of every type that is either some
>>    exclusively specified type, or a subtype of some inclusively
>>    specified type.
>>3) Let TPR be the result of evaluating the <type predicate>.
>>
>>Case:
>>a) If V is the null value, then TPR is Unknown.
>>b) If the most specific type of V is a member of ST, then TPR is True .
>>c) Otherwise, TPR is False
>>
Index: doc/src/sgml/reference.sgml
===================================================================
RCS file: /opt/src/cvs/pgsql-server/doc/src/sgml/reference.sgml,v
retrieving revision 1.45
diff -c -r1.45 reference.sgml
*** doc/src/sgml/reference.sgml    27 Jun 2003 14:45:25 -0000    1.45
--- doc/src/sgml/reference.sgml    8 Aug 2003 05:16:26 -0000
***************
*** 108,113 ****
--- 108,114 ----
     &fetch;
     &grant;
     &insert;
+    &isof;
     &listen;
     &load;
     &lock;
Index: doc/src/sgml/ref/allfiles.sgml
===================================================================
RCS file: /opt/src/cvs/pgsql-server/doc/src/sgml/ref/allfiles.sgml,v
retrieving revision 1.54
diff -c -r1.54 allfiles.sgml
*** doc/src/sgml/ref/allfiles.sgml    27 Jun 2003 14:45:25 -0000    1.54
--- doc/src/sgml/ref/allfiles.sgml    8 Aug 2003 04:45:35 -0000
***************
*** 76,81 ****
--- 76,82 ----
  <!entity fetch              system "fetch.sgml">
  <!entity grant              system "grant.sgml">
  <!entity insert             system "insert.sgml">
+ <!entity isof               system "isof.sgml">
  <!entity listen             system "listen.sgml">
  <!entity load               system "load.sgml">
  <!entity lock               system "lock.sgml">
Index: doc/src/sgml/ref/isof.sgml
===================================================================
RCS file: doc/src/sgml/ref/isof.sgml
diff -N doc/src/sgml/ref/isof.sgml
*** /dev/null    1 Jan 1970 00:00:00 -0000
--- doc/src/sgml/ref/isof.sgml    8 Aug 2003 05:20:46 -0000
***************
*** 0 ****
--- 1,84 ----
+ <refentry id="SQL-ISOF">
+  <refmeta>
+   <refentrytitle id="SQL-ISOF-TITLE">IS OF</refentrytitle>
+   <refmiscinfo>SQL - Language Statements</refmiscinfo>
+  </refmeta>
+
+  <refnamediv>
+   <refname>IS OF</refname>
+   <refpurpose>specify a type test</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+ <synopsis>
+ <replaceable class="parameter">expression</replaceable> IS [ NOT ] OF ( <replaceable
class="PARAMETER">typename</replaceable>[,... ] ) 
+ </synopsis>
+  </refsynopsisdiv>
+
+  <refsect1 id="sql-isof-description">
+   <title>Description</title>
+
+   <para>
+    This command allows an expression to be checked against a list
+    of data types. It returns TRUE if any data type in the list matches
+    that of the <replaceable class="parameter">expression</replaceable>,
+    FALSE otherwise.
+   </para>
+
+  </refsect1>
+
+  <refsect1>
+   <title>Examples</title>
+ <programlisting>
+ regression=# SELECT 1::int4 IS OF (int4, int8);
+  ?column?
+ ----------
+  t
+ (1 row)
+
+ regression=# SELECT 1::int4 IS OF (text);
+  ?column?
+ ----------
+  f
+ (1 row)
+
+ regression=# SELECT ARRAY['a'] IS OF (text[]);
+  ?column?
+ ----------
+  t
+ (1 row)
+ </programlisting>
+  </refsect1>
+
+  <refsect1 id="sql-isof-compat">
+   <title>Compatibility</title>
+
+   <para>
+    The SQL standard specifies that <literal>IS OF</literal>
+    must result in <literal>UNKNOWN</literal> if
+    <replaceable class="parameter">expression</replaceable> is
+    <literal>NULL</literal>. The <productname>PostgreSQL</productname>
+    implementation results in <literal>TRUE</literal> or <literal>FALSE</literal>
+    as appropriate based on the data type assigned by the planner.
+    Otherwise, this command is fully conforming.
+   </para>
+  </refsect1>
+
+ </refentry>
+
+ <!-- Keep this comment at the end of the file
+ Local variables:
+ mode: sgml
+ sgml-omittag:nil
+ sgml-shorttag:t
+ sgml-minimize-attributes:nil
+ sgml-always-quote-attributes:t
+ sgml-indent-step:1
+ sgml-indent-data:t
+ sgml-parent-document:nil
+ sgml-default-dtd-file:"../reference.ced"
+ sgml-exposed-tags:nil
+ sgml-local-catalogs:"/usr/lib/sgml/catalog"
+ sgml-local-ecat-files:nil
+ End:
+ -->

Re: [HACKERS] IS OF

From
Peter Eisentraut
Date:
Joe Conway writes:

> Bruce Momjian wrote:
> > Can someone suggest where to document IS OF, and either document it's
> > non-standard behavior or supply patch?
> >

I suggest I should not be documented until it's fixed.

> Doc patch attached for IS OF. Please apply.

That is not the right place for it.  IS OF is an operator, not an SQL
command.

--
Peter Eisentraut   peter_e@gmx.net

Re: [HACKERS] IS OF

From
Joe Conway
Date:
Peter Eisentraut wrote:
> I suggest I should not be documented until it's fixed.
>
>>Doc patch attached for IS OF. Please apply.
>
> That is not the right place for it.  IS OF is an operator, not an SQL
> command.
>

OK. If the attached patch is acceptable/applied, I'll fix and resend the
doc patch.

Joe
Index: src/backend/parser/parse_expr.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/parser/parse_expr.c,v
retrieving revision 1.160
diff -c -r1.160 parse_expr.c
*** src/backend/parser/parse_expr.c    4 Aug 2003 02:40:01 -0000    1.160
--- src/backend/parser/parse_expr.c    8 Aug 2003 21:53:25 -0000
***************
*** 45,51 ****
  static Node *transformColumnRef(ParseState *pstate, ColumnRef *cref);
  static Node *transformIndirection(ParseState *pstate, Node *basenode,
                       List *indirection);
!

  /*
   * Initialize for parsing a new query.
--- 44,50 ----
  static Node *transformColumnRef(ParseState *pstate, ColumnRef *cref);
  static Node *transformIndirection(ParseState *pstate, Node *basenode,
                       List *indirection);
! static bool exprIsNullConstantTypeCast(Node *arg);

  /*
   * Initialize for parsing a new query.
***************
*** 394,421 ****
                              Node       *lexpr = transformExpr(pstate,
                                                                a->lexpr);

-                             ltype = exprType(lexpr);
-                             foreach(telem, (List *) a->rexpr)
-                             {
-                                 rtype = LookupTypeName(lfirst(telem));
-                                 matched = (rtype == ltype);
-                                 if (matched)
-                                     break;
-                             }
-
-                             /*
-                              * Expect two forms: equals or not equals.
-                              * Flip the sense of the result for not
-                              * equals.
-                              */
-                             if (strcmp(strVal(lfirst(a->name)), "!=") == 0)
-                                 matched = (!matched);
-
                              n = makeNode(A_Const);
                              n->val.type = T_String;
-                             n->val.val.str = (matched ? "t" : "f");
                              n->typename = SystemTypeName("bool");

                              result = transformExpr(pstate, (Node *) n);
                          }
                          break;
--- 393,426 ----
                              Node       *lexpr = transformExpr(pstate,
                                                                a->lexpr);

                              n = makeNode(A_Const);
                              n->val.type = T_String;
                              n->typename = SystemTypeName("bool");

+                             if (exprIsNullConstantTypeCast(a->lexpr))
+                                 n->val.val.str = "f";
+                             else
+                             {
+                                 ltype = exprType(lexpr);
+                                 foreach(telem, (List *) a->rexpr)
+                                 {
+                                     rtype = LookupTypeName(lfirst(telem));
+                                     matched = (rtype == ltype);
+                                     if (matched)
+                                         break;
+                                 }
+
+                                 /*
+                                  * Expect two forms: equals or not equals.
+                                  * Flip the sense of the result for not
+                                  * equals.
+                                  */
+                                 if (strcmp(strVal(lfirst(a->name)), "!=") == 0)
+                                     matched = (!matched);
+
+                                 n->val.val.str = (matched ? "t" : "f");
+                             }
+
                              result = transformExpr(pstate, (Node *) n);
                          }
                          break;
***************
*** 1567,1570 ****
--- 1572,1588 ----
                          format_type_be(targetType))));

      return expr;
+ }
+
+
+ /* exprIsNullConstantTypeCast()
+  * Recursively test whether an a_expr is a typecast NULL constant or not.
+  */
+ static bool
+ exprIsNullConstantTypeCast(Node *arg)
+ {
+     if (arg && IsA(arg, TypeCast))
+         return exprIsNullConstantTypeCast(((TypeCast *) arg)->arg);
+
+     return exprIsNullConstant(arg);
  }

Re: [HACKERS] IS OF

From
Joe Conway
Date:
Joe Conway wrote:
> Peter Eisentraut wrote:
>> I suggest I should not be documented until it's fixed.
>>
>>> Doc patch attached for IS OF. Please apply.
>>
>> That is not the right place for it.  IS OF is an operator, not an SQL
>> command.
>
> OK. If the attached patch is acceptable/applied, I'll fix and resend the
> doc patch.
>

Hmmm, looks like I was a bit quick on the draw:

regression=# select f1 is null, f2 is null, f3 is null from foo;
  ?column? | ?column? | ?column?
----------+----------+----------
  f        | t        | f
  t        | f        | f
  f        | f        | t
(3 rows)

regression=# select f1 is of (int), f2 is of (text), f3 is of (float8)
from foo;
  ?column? | ?column? | ?column?
----------+----------+----------
  t        | t        | t
  t        | t        | t
  t        | t        | t
(3 rows)


It worked correctly for constants, but not fields from a table :(

Back to the drawing board (suggestions welcomed).

Joe


Re: [HACKERS] IS OF

From
Tom Lane
Date:
Joe Conway <mail@joeconway.com> writes:
> OK. If the attached patch is acceptable/applied, I'll fix and resend the
> doc patch.

I'm unconvinced that the parse-time-constant implementation Lockhart
started has anything whatever to do with the semantics the SQL99 spec
has in mind.  In the first place, the spec seems to expect that the
lefthand side will actually be evaluated.  Checking for a NULL constant
doesn't cover cases where the LHS returns NULL dynamically; let alone
cases where it would cause an error.

I also get the impression that they think the result may vary at
runtime.  This is not totally impossible in Postgres, either --- you
could imagine that the LHS is a tuple from some inheritance tree, and
the IS OF query really amounts to asking which child table the tuple
came from.

Also, simple equality checks on the type OIDs don't cover the inheritance
cases (I think a child tuple should be said to be IS OF the tuple type
of its parent).  And what about domains --- should we say a domain type
IS OF its base type?

            regards, tom lane

Re: [HACKERS] IS OF

From
Joe Conway
Date:
Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>
>>OK. If the attached patch is acceptable/applied, I'll fix and resend the
>>doc patch.
>
> I'm unconvinced that the parse-time-constant implementation Lockhart
> started has anything whatever to do with the semantics the SQL99 spec
> has in mind.

Yeah - I've realized this is quite a bit harder than it seemed on the
surface. However it is still useful, as is, when working with
polymorphic functions.

So do we rip it out, leave it undocumented, or document it including the
deviation from spec?

Joe


Re: [HACKERS] IS OF

From
Tom Lane
Date:
Joe Conway <mail@joeconway.com> writes:
> Tom Lane wrote:
>> I'm unconvinced that the parse-time-constant implementation Lockhart
>> started has anything whatever to do with the semantics the SQL99 spec
>> has in mind.

> Yeah - I've realized this is quite a bit harder than it seemed on the
> surface. However it is still useful, as is, when working with
> polymorphic functions.

In fact you could argue that our current behavior is *more* useful than
what the spec says for polymorphics.  You would not want the special
case for NULLs, in most cases, I'd think.  NULLs have perfectly well
defined datatype.

However, it troubles me to be using a spec-defined syntax for a behavior
that is not standard.  I'd prefer to change the syntax if we are going
to keep the behavior.  That probably puts it in the "too late for 7.4"
category.  So I'm inclined to follow the path of leaving it undocumented
for now, implementing a new syntax in 7.5, and documenting it under that
syntax then.

            regards, tom lane

Re: [HACKERS] IS OF

From
Joe Conway
Date:
Tom Lane wrote:
> In fact you could argue that our current behavior is *more* useful than
> what the spec says for polymorphics.  You would not want the special
> case for NULLs, in most cases, I'd think.  NULLs have perfectly well
> defined datatype.

That's actually exactly what I was thinking.

> However, it troubles me to be using a spec-defined syntax for a behavior
> that is not standard.  I'd prefer to change the syntax if we are going
> to keep the behavior.  That probably puts it in the "too late for 7.4"
> category.  So I'm inclined to follow the path of leaving it undocumented
> for now, implementing a new syntax in 7.5, and documenting it under that
> syntax then.
>

Sounds good to me.

Thanks,

Joe