Thread: BUG #4434: Error inserting into view - unrecognized node type: 313

The following bug has been logged online:

Bug reference:      4434
Logged by:          Dean Rasheed
Email address:      dean_rasheed@hotmail.com
PostgreSQL version: 8.3
Operating system:   SuSE 10.3 64-bit
Description:        Error inserting into view - unrecognized node type: 313
Details:

I'm getting an error when trying to insert into a view, using a
rule. As far as I can work out, it only errors if my insert statement
tries to insert multiple rows, and any of the rows contains a
sub-query. Here is a simple made-up example:

DROP TABLE IF EXISTS foo CASCADE;
CREATE TABLE foo(a int, b int);
CREATE VIEW foo_v AS SELECT * FROM foo;
CREATE RULE foo_r AS ON INSERT TO foo_v DO INSTEAD INSERT INTO foo
VALUES(NEW.a, NEW.b);
INSERT INTO foo_v VALUES ((SELECT 1), (SELECT 2)), ((SELECT 3), (SELECT
4));

ERROR:  unrecognized node type: 313

Inserting the rows one at a time works OK, as does inserting into a
table, or inserting both rows together but without using sub-queries.

I've tested this with 8.3 and a fairly recent CVS version.

Dean.

"Dean Rasheed" <dean_rasheed@hotmail.com> writes:
> CREATE TABLE foo(a int, b int);
> CREATE VIEW foo_v AS SELECT * FROM foo;
> CREATE RULE foo_r AS ON INSERT TO foo_v DO INSTEAD INSERT INTO foo
> VALUES(NEW.a, NEW.b);
> INSERT INTO foo_v VALUES ((SELECT 1), (SELECT 2)), ((SELECT 3), (SELECT 4));

> ERROR:  unrecognized node type: 313

It looks like the parser's code path for multi-row VALUES is neglecting
to detect sublinks and set pstate->p_hasSubLinks.  I'm too tired to look
closer tonight; anyone want to poke into it?

            regards, tom lane

Tom Lane wrote:
> "Dean Rasheed" <dean_rasheed@hotmail.com> writes:
>> CREATE TABLE foo(a int, b int);
>> CREATE VIEW foo_v AS SELECT * FROM foo;
>> CREATE RULE foo_r AS ON INSERT TO foo_v DO INSTEAD INSERT INTO foo
>> VALUES(NEW.a, NEW.b);
>> INSERT INTO foo_v VALUES ((SELECT 1), (SELECT 2)), ((SELECT 3), (SELECT 4));
>
>> ERROR:  unrecognized node type: 313
>
> It looks like the parser's code path for multi-row VALUES is neglecting
> to detect sublinks and set pstate->p_hasSubLinks.  I'm too tired to look
> closer tonight; anyone want to poke into it?

I think the parser is OK, but the p_hasSubLinks is lost in the rewrite
phase. In ResolveNew, we set p_hasSubLinks whenever a Var with a SubLink
is found. In case of Values RTE, however, there's no Vars, but plain
SubLink nodes. This patch seems to fix it:

--- src/backend/rewrite/rewriteManip.c
+++ src/backend/rewrite/rewriteManip.c
@@ -1112,6 +1112,12 @@ ResolveNew_mutator(Node *node, ResolveNew_context
*context)
          context->sublevels_up--;
          return (Node *) newnode;
      }
+    else if (IsA(node, SubLink))
+    {
+        /* Report it if we are adding a sublink to query */
+        context->inserted_sublink = true;
+        /* fall through to copy the expr normally */
+    }
      return expression_tree_mutator(node, ResolveNew_mutator,
                                     (void *) context);
  }

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

On Tue, Sep 23, 2008 at 10:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Dean Rasheed" <dean_rasheed@hotmail.com> writes:
>> CREATE TABLE foo(a int, b int);
>> CREATE VIEW foo_v AS SELECT * FROM foo;
>> CREATE RULE foo_r AS ON INSERT TO foo_v DO INSTEAD INSERT INTO foo
>> VALUES(NEW.a, NEW.b);
>> INSERT INTO foo_v VALUES ((SELECT 1), (SELECT 2)), ((SELECT 3), (SELECT 4));
>
>> ERROR:  unrecognized node type: 313
>
> It looks like the parser's code path for multi-row VALUES is neglecting
> to detect sublinks and set pstate->p_hasSubLinks.  I'm too tired to look
> closer tonight; anyone want to poke into it?
>
>                        regards, tom lane

The below fixes it for me...  Its probably in the wrong place,  and
does not even try to do any detection... but here it is anyway

*** a/src/backend/parser/analyze.c
--- b/src/backend/parser/analyze.c
***************
*** 538,543 **** transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
--- 538,545 ----
           */
          List       *valuesLists = selectStmt->valuesLists;

+         pstate->p_hasSubLinks = true;
+
          Assert(list_length(valuesLists) == 1);

          /* Do basic expression transformation (same as a ROW() expr) */

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> Tom Lane wrote:
>> It looks like the parser's code path for multi-row VALUES is neglecting
>> to detect sublinks and set pstate->p_hasSubLinks.  I'm too tired to look
>> closer tonight; anyone want to poke into it?

> I think the parser is OK, but the p_hasSubLinks is lost in the rewrite
> phase.

Yeah, clearly so after comparing parser and rewriter outputs.

> In ResolveNew, we set p_hasSubLinks whenever a Var with a SubLink
> is found. In case of Values RTE, however, there's no Vars, but plain
> SubLink nodes. This patch seems to fix it:

No, that's not really where the problem is.  After tracing through it,
it seems that the fault is with rewriteRuleAction().  That pastes the
source query's rtable into the rule action's query, but neglects to
consider the possibility that this change is introducing sublinks.
There is code there that will notice sublinks added from the source
query's jointree (ie, sublinks that are in WHERE or JOIN/ON), but
we are missing out on sublinks in VALUES RTEs ... and FUNCTION RTEs
too I suppose, so this goes back a ways.  (The code was correct when
originally written, but now that we have expressions in some types of
RTEs, it's not.)

Will fix.

            regards, tom lane