Thread: Re: [HACKERS] IS OF
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: + -->
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
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); }
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
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
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
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
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