Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types

[Andres Freund]

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

[Amit Langote]

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

[Peter Geoghegan]

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 Geoghegan]

On Wed, Aug 5, 2015 at 12:58 AM, Amit Langote
langote_amit...@lab.ntt.co.jp 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

[Amit Langote]

On 2015-08-05 AM 06:44, Peter Geoghegan wrote:
 On Tue, Aug 4, 2015 at 2:29 AM, Amit Langote
 langote_amit...@lab.ntt.co.jp 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

[Amit Langote]

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

[Geoff Winkless]

On 4 August 2015 at 09:30, Amit Langote langote_amit...@lab.ntt.co.jp
wrote:

 On 2015-08-04 AM 02:57, Peter Geoghegan wrote:
  On Mon, Aug 3, 2015 at 8:53 AM, Geoff Winkless pgsqlad...@geoff.dj
 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

[Amit Langote]

On 2015-08-04 AM 02:57, Peter Geoghegan wrote:
 On Mon, Aug 3, 2015 at 8:53 AM, Geoff Winkless pgsqlad...@geoff.dj 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

[Peter Geoghegan]

On Tue, Aug 4, 2015 at 2:29 AM, Amit Langote
langote_amit...@lab.ntt.co.jp 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

[HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types

[Geoff Winkless]

Hi

We've come across a weirdness with ON CONFLICT, where UPSERTing a smallint
value produces an error:

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.

If you change the SET to smallval=0 the problem goes away, although using
SET smallval=CAST(EXCLUDED.smallval AS smallint) - or indeed AS int -
doesn't help at all.

If I create a copy of the table using

CREATE mytab (LIKE brokentab INCLUDING ALL);
INSERT INTO mytab SELECT * FROM brokentab;

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?

Linux Centos 6.5, kernel 2.6.32-431.el6.i686, pgsql alpha1, built from
source using gcc 4.4.7.

Thanks

Geoff

Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types

[Peter Geoghegan]

On Mon, Aug 3, 2015 at 8:53 AM, Geoff Winkless pgsqlad...@geoff.dj 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

Re: [HACKERS] ON CONFLICT DO UPDATE using EXCLUDED.column gives an error about mismatched types

[Peter Geoghegan]

On Mon, Aug 3, 2015 at 8:53 AM, Geoff Winkless pgsqlad...@geoff.dj 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