Re: "SELECT ... FROM DUAL" is not quite as silly as it appears

Mark Dilger <hornschnorter(at)gmail(dot)com> writes:
> Patch applies cleanly on master (b238527664ec6f6c9d00dba4cc2f3dab1c8b8b04), compiles, and passes both 'make check-world' and 'make installcheck-world'.

Thanks for reviewing!

> The patch includes changes to the expected output of a few tests, and adds new code comments and changes existing code comments, but I did not notice any new tests or new documentation to specifically test or explain the behavioral change this patch is intended to introduce. None of the code comments seem to adequately explain what an RTE_RESULT is and when it would be used. This information can be gleaned with some difficulty by reading every file containing RTE_RESULT, but that seems rather unfriendly.

Well, there's no user-facing documentation because it's not a user-facing
feature; it just exists to make some things simpler inside the planner.

I will admit that the comment for RTE_RESULT in parsenodes.h is a bit
vague; that's mostly because when I started on this patch, it wasn't clear
to me whether or not to have RTE_RESULT present from the beginning (i.e.
have the parser create one) or let the planner inject them. I ended up
doing the latter, so the attached update of the patch now says

@@ -950,7 +950,10 @@ typedef enum RTEKind
RTE_TABLEFUNC, /* TableFunc(.., column list) */
RTE_VALUES, /* VALUES (<exprlist>), (<exprlist>), ... */
RTE_CTE, /* common table expr (WITH list element) */
- RTE_NAMEDTUPLESTORE /* tuplestore, e.g. for AFTER triggers */
+ RTE_NAMEDTUPLESTORE, /* tuplestore, e.g. for AFTER triggers */
+ RTE_RESULT /* RTE represents an empty FROM clause; such
+ * RTEs are added by the planner, they're not
+ * present during parsing or rewriting */
} RTEKind;

typedef struct RangeTblEntry

I'm not sure if that's enough to address your concern or not. But none
of the other RTEKinds are documented much more than this, either ...

> As an example of where I could use a bit more documentation, see
> src/backend/rewrite/rewriteHandler.c circa line 447; I don't know why
> the switch statement lacks a case for RTE_RESULT. Why would RTE_VALUES
> contain bare expressions but RTE_RESULT would not?

Well, it just doesn't. The comments in struct RangeTblEntry are pretty
clear about which fields apply to which RTE kinds, and none of the ones
containing subexpressions are valid for RTE_RESULT. As to *why* it's
like that, it's because an empty FROM clause doesn't produce any columns,
by definition.

> Does this mean that
> INSERT INTO mytable VALUES ('foo', 'bar');
> differs from
> SELECT 'foo', 'bar';
> in terms of whether 'foo' and 'bar' are bare expressions?

Well, it does if there are multiple rows of VALUES items; look at the
parser's transformInsertStmt, which does things differently for a
single-row VALUES than multiple rows. We only create a VALUES RTE
for the multi-rows case, for which "SELECT 'foo', 'bar'" doesn't work.

Attached is an updated patch that responds to your comments and
Alexander's, and also adds a test case for EvalPlanQual involving
a RTE_RESULT RTE, because I got worried about whether that really
worked.

regards, tom lane