Skip to content

Commit

Permalink
Go back to putting inference WHERE clause in parentheses
Browse files Browse the repository at this point in the history
Andres felt it was important that the inference specification match the
CREATE INDEX syntax as closely as possible, including having any WHERE
clause follow the parentheses (containing a list of columns/expressions
to be indexed).  The problem with this seems to be that it more or less
necessitates making both IGNORE and UPDATE fully reserved keywords in
order to avoid an ambiguity, which we prefer not to do.

Andres said:

"""
But more importantly with multiple columns for stuff like ON CONFLICT
(a, b WHERE foo) it's unclear where the WHERE is actually attached to.
We have that problem with aggregates and it repeatedly caused confusion.

"""

I don't think this quite follows, though.  In fact, the CREATE INDEX
syntax treats plain columns and simple function expressions in a special
way -- in general, individual attributes must appear between their own
parentheses.  So this doesn't work:

"""
postgres=# insert into upsert values(3, 'Bat') on conflict (1) ignore;
ERROR:  42601: syntax error at or near "1"
LINE 1: insert into upsert values(3, 'Bat') on conflict (1) ignore;
                                                         ^
"""

But this does:

"""
postgres=# insert into upsert values(3, 'Bat') on conflict ((1)) ignore;

"""

Once you consider that the parser actually treats simple column
references/functions in a special way, where the WHERE clause is
attached to becomes less troubling.  It also happens to be convenient to
do things this way, but that is secondary.  A DO UPDATE/DO NOTHING
syntax seems more unappealing.

This change is provisional;  maybe it will be reverted when a better
solution is found.  But let's go this way until a better solution is
actually proposed.
  • Loading branch information
petergeoghegan committed Apr 25, 2015
1 parent 0d81bd9 commit 2a5d80b
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 16 deletions.
4 changes: 2 additions & 2 deletions doc/src/sgml/ref/insert.sgml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ PostgreSQL documentation
[ WITH [ RECURSIVE ] <replaceable class="parameter">with_query</replaceable> [, ...] ]
INSERT INTO <replaceable class="PARAMETER">table_name</replaceable> [ ( <replaceable class="PARAMETER">column_name</replaceable> [, ...] ) ]
{ DEFAULT VALUES | VALUES ( { <replaceable class="PARAMETER">expression</replaceable> | DEFAULT } [, ...] ) [, ...] | <replaceable class="PARAMETER">query</replaceable> }
[ ON CONFLICT [ ( { <replaceable class="parameter">column_name_index</replaceable> | ( <replaceable class="parameter">expression_index</replaceable> ) } [ COLLATE <replaceable class="parameter">collation</replaceable> ] [ <replaceable class="parameter">opclass</replaceable> ] [, ...] ) [ WHERE <replaceable class="PARAMETER">index_predicate</replaceable> ] ]
[ ON CONFLICT [ ( { <replaceable class="parameter">column_name_index</replaceable> | ( <replaceable class="parameter">expression_index</replaceable> ) } [ COLLATE <replaceable class="parameter">collation</replaceable> ] [ <replaceable class="parameter">opclass</replaceable> ] [, ...] [ WHERE <replaceable class="PARAMETER">index_predicate</replaceable> ] ) ]
{ IGNORE | UPDATE
SET { <replaceable class="PARAMETER">column_name</replaceable> = { <replaceable class="PARAMETER">expression</replaceable> | DEFAULT } |
( <replaceable class="PARAMETER">column_name</replaceable> [, ...] ) = ( { <replaceable class="PARAMETER">expression</replaceable> | DEFAULT } [, ...] )
Expand Down Expand Up @@ -681,7 +681,7 @@ INSERT INTO employees_log SELECT *, current_timestamp FROM upd;
-- just use a regular unique constraint on did if that was
-- all that was available.
INSERT INTO distributors (did, dname) VALUES (9, 'Antwerp Design')
ON CONFLICT (did) WHERE is_active IGNORE;
ON CONFLICT (did WHERE is_active) IGNORE;
</programlisting>
</para>
</refsect1>
Expand Down
6 changes: 2 additions & 4 deletions src/backend/parser/gram.y
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,6 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%nonassoc IS ISNULL NOTNULL /* IS sets precedence for IS NULL, etc */
%nonassoc '<' '>' '=' LESS_EQUALS GREATER_EQUALS NOT_EQUALS
%nonassoc BETWEEN IN_P LIKE ILIKE SIMILAR NOT_LA
%nonassoc UPDATE
%nonassoc IGNORE_P
%nonassoc ESCAPE /* ESCAPE must be just above LIKE/ILIKE/SIMILAR */
%nonassoc OVERLAPS
%left POSTFIXOP /* dummy for postfix Op rules */
Expand Down Expand Up @@ -9436,11 +9434,11 @@ opt_on_conflict:
;

opt_conf_expr:
'(' index_params ')' where_clause
'(' index_params where_clause ')'
{
$$ = makeNode(InferClause);
$$->indexElems = $2;
$$->whereClause = $4;
$$->whereClause = $3;
$$->location = @1;
}
| /*EMPTY*/
Expand Down
10 changes: 5 additions & 5 deletions src/test/regress/expected/insert_conflict.out
Original file line number Diff line number Diff line change
Expand Up @@ -311,16 +311,16 @@ drop index fruit_index;
--
create unique index partial_key_index on insertconflicttest(key) where fruit like '%berry';
-- Succeeds
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' update set fruit = excluded.fruit;
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and fruit = 'inconsequential' ignore;
insert into insertconflicttest values (23, 'Blackberry') on conflict (key where fruit like '%berry') update set fruit = excluded.fruit;
insert into insertconflicttest values (23, 'Blackberry') on conflict (key where fruit like '%berry' and fruit = 'inconsequential') ignore;
-- fails
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) update set fruit = excluded.fruit;
ERROR: could not infer which unique index to use from expressions/columns and predicate provided for ON CONFLICT
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' or fruit = 'consequential' ignore;
insert into insertconflicttest values (23, 'Blackberry') on conflict (key where fruit like '%berry' or fruit = 'consequential') ignore;
ERROR: could not infer which unique index to use from expressions/columns and predicate provided for ON CONFLICT
insert into insertconflicttest values (23, 'Blackberry') on conflict (fruit) where fruit like '%berry' update set fruit = excluded.fruit;
insert into insertconflicttest values (23, 'Blackberry') on conflict (fruit where fruit like '%berry') update set fruit = excluded.fruit;
ERROR: could not infer which unique index to use from expressions/columns and predicate provided for ON CONFLICT
insert into insertconflicttest values (23, 'Uncovered by Index') on conflict (key) where fruit like '%berry' ignore;
insert into insertconflicttest values (23, 'Uncovered by Index') on conflict (key where fruit like '%berry') ignore;
ERROR: inferred arbiter partial unique index's predicate does not cover tuple proposed for insertion
DETAIL: ON CONFLICT inference clause implies that the tuple proposed for insertion must be covered by the predicate of partial index "partial_key_index".
drop index partial_key_index;
Expand Down
10 changes: 5 additions & 5 deletions src/test/regress/sql/insert_conflict.sql
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,14 @@ drop index fruit_index;
create unique index partial_key_index on insertconflicttest(key) where fruit like '%berry';

-- Succeeds
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' update set fruit = excluded.fruit;
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and fruit = 'inconsequential' ignore;
insert into insertconflicttest values (23, 'Blackberry') on conflict (key where fruit like '%berry') update set fruit = excluded.fruit;
insert into insertconflicttest values (23, 'Blackberry') on conflict (key where fruit like '%berry' and fruit = 'inconsequential') ignore;

-- fails
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) update set fruit = excluded.fruit;
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' or fruit = 'consequential' ignore;
insert into insertconflicttest values (23, 'Blackberry') on conflict (fruit) where fruit like '%berry' update set fruit = excluded.fruit;
insert into insertconflicttest values (23, 'Uncovered by Index') on conflict (key) where fruit like '%berry' ignore;
insert into insertconflicttest values (23, 'Blackberry') on conflict (key where fruit like '%berry' or fruit = 'consequential') ignore;
insert into insertconflicttest values (23, 'Blackberry') on conflict (fruit where fruit like '%berry') update set fruit = excluded.fruit;
insert into insertconflicttest values (23, 'Uncovered by Index') on conflict (key where fruit like '%berry') ignore;

drop index partial_key_index;

Expand Down

0 comments on commit 2a5d80b

Please sign in to comment.