Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
Hi, > db=# INSERT INTO brokentab(id, k1,k2,k3,k4,k5,k6,k7, smallval) VALUES > (5,0,0,0,1,0,1,0, 0) ON CONFLICT (id, k1,k2,k3,k4,k5,k6,k7) DO UPDATE SET > smallval=EXCLUDED.smallval; > ERROR: attribute 29 has wrong type > DETAIL: Table has type integer, but query expects smallint. I pushed a fix for the issue. Could you verify that your original problem does not exist anymore? Thanks for testing Geoff, thanks for helping to nail this down Amit and Peter. Regards, Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
On Wed, Sep 2, 2015 at 1:18 AM, Amit Langote wrote: > Did you get around to making a patch for this? I've worked on it inconsistently. I'll pick this up again soon. I may take the opportunity to talk this over with Andres in person when we meet at Postgres Open shortly. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
Peter, On 2015-08-11 AM 07:37, Peter Geoghegan wrote: > What I'm going to do is roll this into my own pending patch to fix the > issue with wholerow vars, which is also down to a problem with the > excluded targetlist initially generated by calling expandRelAttrs(). > Andres might want to take an alternative approach with that, so a > consolidation makes sense. Did you get around to making a patch for this? Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
On Wed, Aug 5, 2015 at 12:58 AM, Amit Langote wrote: > I forgot mentioning one thing later yesterday. The way exclRelTlist is > initialized, all the way in the beginning (transformOnConflictClause), is > most probably to blame. It uses expandRelAttrs() for other valid reasons; > but within it, expandRTE() is called with 'false' for include_dropped > (columns). I applied the attached (perhaps ugly) fix, and it seemed to fix > the issue. But, I guess you'll be able to come up with some better fix. Thanks for working on this. I think you have this right, but I also don't think we should discuss this fix outside of the context of a wider discussion about the excluded targetlist. What I'm going to do is roll this into my own pending patch to fix the issue with wholerow vars, which is also down to a problem with the excluded targetlist initially generated by calling expandRelAttrs(). Andres might want to take an alternative approach with that, so a consolidation makes sense. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
On 2015-08-05 AM 06:44, Peter Geoghegan wrote: > On Tue, Aug 4, 2015 at 2:29 AM, Amit Langote > wrote: >> Perhaps, it may have to do with how EXCLUDED pseudo-rel's targetlist is >> manipulated through parse-plan stage? > > I think so, yes. > > I'll look into writing a fix for this later in the week. > Just a heads-up. I forgot mentioning one thing later yesterday. The way exclRelTlist is initialized, all the way in the beginning (transformOnConflictClause), is most probably to blame. It uses expandRelAttrs() for other valid reasons; but within it, expandRTE() is called with 'false' for include_dropped (columns). I applied the attached (perhaps ugly) fix, and it seemed to fix the issue. But, I guess you'll be able to come up with some better fix. Thanks, Amit diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index a0dfbf9..cd67c96 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -915,7 +915,7 @@ transformOnConflictClause(ParseState *pstate, * required permissions back. */ exclRelTlist = expandRelAttrs(pstate, exclRte, - exclRelIndex, 0, -1); + exclRelIndex, 0, -1, true); exclRte->requiredPerms = 0; exclRte->selectedCols = NULL; @@ -1312,7 +1312,7 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt) * Generate a targetlist as though expanding "*" */ Assert(pstate->p_next_resno == 1); - qry->targetList = expandRelAttrs(pstate, rte, rtindex, 0, -1); + qry->targetList = expandRelAttrs(pstate, rte, rtindex, 0, -1, false); /* * The grammar allows attaching ORDER BY, LIMIT, and FOR UPDATE to a diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 0b2dacf..98124e4 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -2418,7 +2418,8 @@ expandTupleDesc(TupleDesc tupdesc, Alias *eref, int count, int offset, */ List * expandRelAttrs(ParseState *pstate, RangeTblEntry *rte, - int rtindex, int sublevels_up, int location) + int rtindex, int sublevels_up, int location, + bool include_dropped_cols) { List *names, *vars; @@ -2426,7 +2427,7 @@ expandRelAttrs(ParseState *pstate, RangeTblEntry *rte, *var; List *te_list = NIL; - expandRTE(rte, rtindex, sublevels_up, location, false, + expandRTE(rte, rtindex, sublevels_up, location, include_dropped_cols, &names, &vars); /* @@ -2448,6 +2449,10 @@ expandRelAttrs(ParseState *pstate, RangeTblEntry *rte, false); te_list = lappend(te_list, te); + /* Dropped columns ain't Vars */ + if (!IsA(varnode, Var)) + continue; + /* Require read access to each column */ markVarForSelectPriv(pstate, varnode, rte); } diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 1b3fcd6..6712464 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -1185,7 +1185,7 @@ ExpandAllTables(ParseState *pstate, int location) RTERangeTablePosn(pstate, rte, NULL), 0, - location)); + location, false)); } /* @@ -1253,7 +1253,7 @@ ExpandSingleTable(ParseState *pstate, RangeTblEntry *rte, { /* expandRelAttrs handles permissions marking */ return expandRelAttrs(pstate, rte, rtindex, sublevels_up, - location); + location, false); } else { diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index e2875a0..591e049 100644 --- a/src/include/parser/parse_relation.h +++ b/src/include/parser/parse_relation.h @@ -110,8 +110,8 @@ extern void errorMissingColumn(ParseState *pstate, extern void expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up, int location, bool include_dropped, List **colnames, List **colvars); -extern List *expandRelAttrs(ParseState *pstate, RangeTblEntry *rte, - int rtindex, int sublevels_up, int location); +extern List *expandRelAttrs(ParseState *pstate, RangeTblEntry *rte, int rtindex, + int sublevels_up, int location, bool include_dropped_cols); extern int attnameAttNum(Relation rd, const char *attname, bool sysColOK); extern Name attnumAttName(Relation rd, int attid); extern Oid attnumTypeId(Relation rd, int attid); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
On Tue, Aug 4, 2015 at 2:29 AM, Amit Langote wrote: > Perhaps, it may have to do with how EXCLUDED pseudo-rel's targetlist is > manipulated through parse-plan stage? I think so, yes. I'll look into writing a fix for this later in the week. Thanks for the report, Geoff, and thanks for the analysis Amit. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
On 2015-08-04 PM 05:58, Geoff Winkless wrote: > > Although it seems Amit has defined the problem better than I could, so > this is a bit late to the party (!), yes, the table had been ALTERed after > it was created (looking back through the history, that modification > included at least one DROP COLUMN). > It seems using any columns that used to be after a dropped columns cause EXCLUDE pseudo-relation to misbehave. For example, I observed another symptom: test=# CREATE TABLE upsert_fail_test(a int, b int, c int, d smallint); CREATE TABLE test=# ALTER TABLE upsert_fail_test DROP b; ALTER TABLE test=# ALTER TABLE upsert_fail_test ADD PRIMARY KEY (a, c, d); ALTER TABLE test=# INSERT INTO upsert_fail_test(a, c, d) VALUES (1, 2, 3) ON CONFLICT (a, c, d) DO UPDATE SET c = EXCLUDED.c; INSERT 0 1 test=# INSERT INTO upsert_fail_test(a, c, d) VALUES (1, 2, 3) ON CONFLICT (a, c, d) DO UPDATE SET c = EXCLUDED.c; ERROR: null value in column "c" violates not-null constraint DETAIL: Failing row contains (1, null, 3). Or, the EXCLUDED pseudo-rel failed to deliver '2' produced by the subplan and instead produced a 'null' which I guess was caused by the dropped column 'b'. Perhaps, it may have to do with how EXCLUDED pseudo-rel's targetlist is manipulated through parse-plan stage? Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
On 4 August 2015 at 09:30, Amit Langote wrote: > On 2015-08-04 AM 02:57, Peter Geoghegan wrote: > > On Mon, Aug 3, 2015 at 8:53 AM, Geoff Winkless > wrote: > >> If I create a copy of the table using > >> > >> CREATE mytab (LIKE brokentab INCLUDING ALL); > >> INSERT INTO mytab SELECT * FROM brokentab; > > > > Also, did you drop any columns from the original "brokentab" table > > where the bug can be reproduced? > > > > This seem to be the case. I could reproduce the reported problem: > Although it seems Amit has defined the problem better than I could, so this is a bit late to the party (!), yes, the table had been ALTERed after it was created (looking back through the history, that modification included at least one DROP COLUMN). Thanks Geoff
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
On 2015-08-04 AM 02:57, Peter Geoghegan wrote: > On Mon, Aug 3, 2015 at 8:53 AM, Geoff Winkless wrote: >> If I create a copy of the table using >> >> CREATE mytab (LIKE brokentab INCLUDING ALL); >> INSERT INTO mytab SELECT * FROM brokentab; > > Also, did you drop any columns from the original "brokentab" table > where the bug can be reproduced? > This seem to be the case. I could reproduce the reported problem: test=# CREATE TABLE upsert_fail_test(a int, b int, c smallint); CREATE TABLE test=# ALTER TABLE upsert_fail_test DROP c; ALTER TABLE test=# ALTER TABLE upsert_fail_test ADD c smallint; ALTER TABLE test=# ALTER TABLE upsert_fail_test ADD PRIMARY KEY (a, b, c); ALTER TABLE test=# INSERT INTO upsert_fail_test(a, b, c) VALUES (1, 2, 0) ON CONFLICT (a, b, c) DO UPDATE SET c = EXCLUDED.c; INSERT 0 1 test=# INSERT INTO upsert_fail_test(a, b, c) VALUES (1, 2, 0) ON CONFLICT (a, b, c) DO UPDATE SET b = EXCLUDED.b; INSERT 0 1 test=# INSERT INTO upsert_fail_test(a, b, c) VALUES (1, 2, 0) ON CONFLICT (a, b, c) DO UPDATE SET c = EXCLUDED.c; ERROR: attribute 3 has wrong type DETAIL: Table has type integer, but query expects smallint. FWIW, I tried to look why that happens. It seems during set_plan_refs(), fix_join_expr on the splan->onConflictSet targetlist using EXCLUDE pseudo-rel's targetlist as inner targetlist causes columns c's varattno to be changed to 3, whereas in the actual tuple descriptor it's 4 (dropped and added). Eventually, ExecEvalScalarVar() complains when it finds attno 3 is a dropped attribute. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
On Mon, Aug 3, 2015 at 8:53 AM, Geoff Winkless wrote: > If I create a copy of the table using > > CREATE mytab (LIKE brokentab INCLUDING ALL); > INSERT INTO mytab SELECT * FROM brokentab; Also, did you drop any columns from the original "brokentab" table where the bug can be reproduced? -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types
On Mon, Aug 3, 2015 at 8:53 AM, Geoff Winkless wrote: > the new table does not exhibit the same problem (so I'm assuming it's not > easily reproducible and giving you a creation script isn't going to help). > > VACUUM FULL on the table makes no difference. > > Is there anything you guys can suggest that I can do to help narrow down the > problem? Yes. You should enable all of the following settings: debug_print_parse (boolean) debug_print_rewritten (boolean) debug_print_plan (boolean) debug_pretty_print (boolean) which you can do dynamically from psql. Then post the output for both the failing version (on that table where you can reproduce the issue), and the other table where it seems that you ought to be able to reproduce the problem, but you can't. Maybe that'll show where the problem is. Thanks -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers