public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 4/7] Fix a bug of insertion into an internal partition.
10+ messages / 5 participants
[nested] [flat]
* [PATCH 4/7] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07 amit <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: amit @ 2016-12-13 06:07 UTC (permalink / raw)
Since implicit partition constraints are not inherited, an internal
partition's constraint was not being enforced when targeted directly.
So, include such constraint when setting up leaf partition result
relations for tuple-routing.
Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
src/backend/commands/copy.c | 1 -
src/backend/commands/tablecmds.c | 1 -
src/backend/executor/execMain.c | 41 ++++++++++++++++++++++++++++--------
src/include/executor/executor.h | 1 -
src/test/regress/expected/insert.out | 6 ++++++
src/test/regress/sql/insert.sql | 5 +++++
6 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index afbfb9f6e8..5aa4449e3e 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2429,7 +2429,6 @@ CopyFrom(CopyState cstate)
InitResultRelInfo(resultRelInfo,
cstate->rel,
1, /* dummy rangetable index */
- true, /* do load partition check expression */
NULL,
0);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3c08551d38..2e51124eb3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1323,7 +1323,6 @@ ExecuteTruncate(TruncateStmt *stmt)
InitResultRelInfo(resultRelInfo,
rel,
0, /* dummy rangetable index */
- false,
NULL,
0);
resultRelInfo++;
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6a82c18571..c991a18ce8 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -824,10 +824,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
resultRelationOid = getrelid(resultRelationIndex, rangeTable);
resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+
InitResultRelInfo(resultRelInfo,
resultRelation,
resultRelationIndex,
- true,
NULL,
estate->es_instrument);
resultRelInfo++;
@@ -1218,10 +1218,11 @@ void
InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
Relation partition_root,
int instrument_options)
{
+ List *partition_check = NIL;
+
MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
resultRelInfo->type = T_ResultRelInfo;
resultRelInfo->ri_RangeTableIndex = resultRelationIndex;
@@ -1257,14 +1258,38 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
- if (load_partition_check)
- resultRelInfo->ri_PartitionCheck =
- RelationGetPartitionQual(resultRelationDesc);
/*
- * The following gets set to NULL unless we are initializing leaf
- * partitions for tuple-routing.
+ * If partition_root has been specified, that means we are builiding the
+ * ResultRelationInfo for one of its leaf partitions. In that case, we
+ * need *not* initialize the leaf partition's constraint, but rather the
+ * the partition_root's (if any). We must do that explicitly like this,
+ * because implicit partition constraints are not inherited like user-
+ * defined constraints and would fail to be enforced by ExecConstraints()
+ * after a tuple is routed to a leaf partition.
*/
+ if (partition_root)
+ {
+ /*
+ * Root table itself may or may not be a partition; partition_check
+ * would be NIL in the latter case.
+ */
+ partition_check = RelationGetPartitionQual(partition_root);
+
+ /*
+ * This is not our own partition constraint, but rather an ancestor's.
+ * So any Vars in it bear the ancestor's attribute numbers. We must
+ * switch them to our own.
+ */
+ if (partition_check != NIL)
+ partition_check = map_partition_varattnos(partition_check,
+ resultRelationDesc,
+ partition_root);
+ }
+ else
+ partition_check = RelationGetPartitionQual(resultRelationDesc);
+
+ resultRelInfo->ri_PartitionCheck = partition_check;
resultRelInfo->ri_PartitionRoot = partition_root;
}
@@ -1328,7 +1353,6 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
- true,
NULL,
estate->es_instrument);
estate->es_trig_target_relations =
@@ -3133,7 +3157,6 @@ ExecSetupPartitionTupleRouting(Relation rel,
InitResultRelInfo(leaf_part_rri,
partrel,
1, /* dummy */
- false,
rel,
0);
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 70ecf108a3..4fed4e101f 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -189,7 +189,6 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
Relation partition_root,
int instrument_options);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index b120954997..6a6aac5a88 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -339,6 +339,12 @@ alter table p add constraint check_b check (b = 3);
insert into p values (1, 2);
ERROR: new row for relation "p11" violates check constraint "check_b"
DETAIL: Failing row contains (1, 2).
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+ERROR: new row for relation "p11" violates partition constraint
+DETAIL: Failing row contains (3, 2).
-- cleanup
drop table p cascade;
NOTICE: drop cascades to 2 other objects
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 3d2fdb92c5..171196df6d 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -200,5 +200,10 @@ alter table p add constraint check_b check (b = 3);
-- after "(1, 2)" is routed to it
insert into p values (1, 2);
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+
-- cleanup
drop table p cascade;
--
2.11.0
--------------AEE611E30AEE235CC4F15C3B
Content-Type: text/x-diff;
name="0005-Add-some-more-tests-for-tuple-routing.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="0005-Add-some-more-tests-for-tuple-routing.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 4/5] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07 amit <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: amit @ 2016-12-13 06:07 UTC (permalink / raw)
Since implicit partition constraints are not inherited, an internal
partition's constraint was not being enforced when targeted directly.
So, include such constraint when setting up leaf partition result
relations for tuple-routing.
---
src/backend/commands/copy.c | 19 +++++++++++++++++--
src/backend/commands/tablecmds.c | 2 +-
src/backend/executor/execMain.c | 22 +++++++++++++++-------
src/backend/executor/nodeModifyTable.c | 12 +++++++++++-
src/include/executor/executor.h | 3 +--
5 files changed, 45 insertions(+), 13 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index e9177491e2..b05055808f 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1411,6 +1411,7 @@ BeginCopy(ParseState *pstate,
int i,
num_parted;
ResultRelInfo *leaf_part_rri;
+ List *partcheck = NIL;
/* Get the tuple-routing information and lock partitions */
cstate->partition_dispatch_info =
@@ -1426,6 +1427,15 @@ BeginCopy(ParseState *pstate,
palloc0(cstate->num_partitions *
sizeof(TupleConversionMap *));
+ /*
+ * If the main target rel is a partition, ExecConstraints() as
+ * applied to each leaf partition must consider its partition
+ * constraint, because unlike explicit constraints, an implicit
+ * partition constraint is not inherited.
+ */
+ if (rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(rel, true);
+
leaf_part_rri = cstate->partitions;
i = 0;
foreach(cell, leaf_parts)
@@ -1449,7 +1459,7 @@ BeginCopy(ParseState *pstate,
InitResultRelInfo(leaf_part_rri,
partrel,
1, /* dummy */
- false, /* no partition constraint check */
+ partcheck,
0);
/* Open partition indices */
@@ -2335,6 +2345,7 @@ CopyFrom(CopyState cstate)
uint64 processed = 0;
bool useHeapMultiInsert;
int nBufferedTuples = 0;
+ List *partcheck = NIL;
#define MAX_BUFFERED_TUPLES 1000
HeapTuple *bufferedTuples = NULL; /* initialize to silence warning */
@@ -2451,6 +2462,10 @@ CopyFrom(CopyState cstate)
hi_options |= HEAP_INSERT_FROZEN;
}
+ /* Don't forget the partition constraints */
+ if (cstate->rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(cstate->rel, true);
+
/*
* We need a ResultRelInfo so we can use the regular executor's
* index-entry-making machinery. (There used to be a huge amount of code
@@ -2460,7 +2475,7 @@ CopyFrom(CopyState cstate)
InitResultRelInfo(resultRelInfo,
cstate->rel,
1, /* dummy rangetable index */
- true, /* do load partition check expression */
+ partcheck,
0);
ExecOpenIndices(resultRelInfo, false);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f94cab60a3..48adeedbe0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1329,7 +1329,7 @@ ExecuteTruncate(TruncateStmt *stmt)
InitResultRelInfo(resultRelInfo,
rel,
0, /* dummy rangetable index */
- false,
+ NIL, /* No need for partition constraints */
0);
resultRelInfo++;
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index d43a204808..963cd2ae43 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -820,13 +820,19 @@ InitPlan(QueryDesc *queryDesc, int eflags)
Index resultRelationIndex = lfirst_int(l);
Oid resultRelationOid;
Relation resultRelation;
+ List *partcheck = NIL;
resultRelationOid = getrelid(resultRelationIndex, rangeTable);
resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+
+ /* Don't forget the partition constraint */
+ if (resultRelation->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(resultRelation, true);
+
InitResultRelInfo(resultRelInfo,
resultRelation,
resultRelationIndex,
- true,
+ partcheck,
estate->es_instrument);
resultRelInfo++;
}
@@ -1216,7 +1222,7 @@ void
InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
+ List *partition_check,
int instrument_options)
{
MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
@@ -1254,10 +1260,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
- if (load_partition_check)
- resultRelInfo->ri_PartitionCheck =
- RelationGetPartitionQual(resultRelationDesc,
- true);
+ resultRelInfo->ri_PartitionCheck = partition_check;
}
/*
@@ -1284,6 +1287,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
ListCell *l;
Relation rel;
MemoryContext oldcontext;
+ List *partcheck = NIL;
/* First, search through the query result relations */
rInfo = estate->es_result_relations;
@@ -1312,6 +1316,10 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
*/
rel = heap_open(relid, NoLock);
+ /* Don't forget the partition constraint */
+ if (rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(rel, true);
+
/*
* Make the new entry in the right context.
*/
@@ -1320,7 +1328,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
- true,
+ partcheck,
estate->es_instrument);
estate->es_trig_target_relations =
lappend(estate->es_trig_target_relations, rInfo);
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index ec440b353d..e236c0e0a7 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1724,6 +1724,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
List *leaf_parts;
ListCell *cell;
ResultRelInfo *leaf_part_rri;
+ List *partcheck = NIL;
/* Get the tuple-routing information and lock partitions */
mtstate->mt_partition_dispatch_info =
@@ -1739,6 +1740,15 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
palloc0(mtstate->mt_num_partitions *
sizeof(TupleConversionMap *));
+ /*
+ * If the main target rel is a partition, ExecConstraints() as
+ * applied to each leaf partition must consider its partition
+ * constraint, because unlike explicit constraints, an implicit
+ * partition constraint is not inherited.
+ */
+ if (rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(rel, true);
+
leaf_part_rri = mtstate->mt_partitions;
i = j = 0;
foreach(cell, leaf_parts)
@@ -1762,7 +1772,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
InitResultRelInfo(leaf_part_rri,
partrel,
1, /* dummy */
- false, /* no partition constraint checks */
+ partcheck,
eflags);
/* Open partition indices (note: ON CONFLICT unsupported)*/
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index b4d09f9564..74213da078 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -73,7 +73,6 @@
#define ExecEvalExpr(expr, econtext, isNull, isDone) \
((*(expr)->evalfunc) (expr, econtext, isNull, isDone))
-
/* Hook for plugins to get control in ExecutorStart() */
typedef void (*ExecutorStart_hook_type) (QueryDesc *queryDesc, int eflags);
extern PGDLLIMPORT ExecutorStart_hook_type ExecutorStart_hook;
@@ -189,7 +188,7 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
+ List *partition_check,
int instrument_options);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
--
2.11.0
--------------87B7BEABA41A98E25D2BEF30
Content-Type: text/x-diff;
name="0005-Fix-a-tuple-routing-bug-in-multi-level-partitioned-t.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0005-Fix-a-tuple-routing-bug-in-multi-level-partitioned-t.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 4/7] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07 amit <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: amit @ 2016-12-13 06:07 UTC (permalink / raw)
Since implicit partition constraints are not inherited, an internal
partition's constraint was not being enforced when targeted directly.
So, include such constraint when setting up leaf partition result
relations for tuple-routing.
Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
src/backend/commands/copy.c | 1 -
src/backend/commands/tablecmds.c | 1 -
src/backend/executor/execMain.c | 41 ++++++++++++++++++++++++++++--------
src/include/executor/executor.h | 1 -
src/test/regress/expected/insert.out | 6 ++++++
src/test/regress/sql/insert.sql | 5 +++++
6 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index afbfb9f6e8..5aa4449e3e 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2429,7 +2429,6 @@ CopyFrom(CopyState cstate)
InitResultRelInfo(resultRelInfo,
cstate->rel,
1, /* dummy rangetable index */
- true, /* do load partition check expression */
NULL,
0);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3c08551d38..2e51124eb3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1323,7 +1323,6 @@ ExecuteTruncate(TruncateStmt *stmt)
InitResultRelInfo(resultRelInfo,
rel,
0, /* dummy rangetable index */
- false,
NULL,
0);
resultRelInfo++;
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6a82c18571..c991a18ce8 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -824,10 +824,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
resultRelationOid = getrelid(resultRelationIndex, rangeTable);
resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+
InitResultRelInfo(resultRelInfo,
resultRelation,
resultRelationIndex,
- true,
NULL,
estate->es_instrument);
resultRelInfo++;
@@ -1218,10 +1218,11 @@ void
InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
Relation partition_root,
int instrument_options)
{
+ List *partition_check = NIL;
+
MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
resultRelInfo->type = T_ResultRelInfo;
resultRelInfo->ri_RangeTableIndex = resultRelationIndex;
@@ -1257,14 +1258,38 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
- if (load_partition_check)
- resultRelInfo->ri_PartitionCheck =
- RelationGetPartitionQual(resultRelationDesc);
/*
- * The following gets set to NULL unless we are initializing leaf
- * partitions for tuple-routing.
+ * If partition_root has been specified, that means we are builiding the
+ * ResultRelationInfo for one of its leaf partitions. In that case, we
+ * need *not* initialize the leaf partition's constraint, but rather the
+ * the partition_root's (if any). We must do that explicitly like this,
+ * because implicit partition constraints are not inherited like user-
+ * defined constraints and would fail to be enforced by ExecConstraints()
+ * after a tuple is routed to a leaf partition.
*/
+ if (partition_root)
+ {
+ /*
+ * Root table itself may or may not be a partition; partition_check
+ * would be NIL in the latter case.
+ */
+ partition_check = RelationGetPartitionQual(partition_root);
+
+ /*
+ * This is not our own partition constraint, but rather an ancestor's.
+ * So any Vars in it bear the ancestor's attribute numbers. We must
+ * switch them to our own.
+ */
+ if (partition_check != NIL)
+ partition_check = map_partition_varattnos(partition_check,
+ resultRelationDesc,
+ partition_root);
+ }
+ else
+ partition_check = RelationGetPartitionQual(resultRelationDesc);
+
+ resultRelInfo->ri_PartitionCheck = partition_check;
resultRelInfo->ri_PartitionRoot = partition_root;
}
@@ -1328,7 +1353,6 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
- true,
NULL,
estate->es_instrument);
estate->es_trig_target_relations =
@@ -3133,7 +3157,6 @@ ExecSetupPartitionTupleRouting(Relation rel,
InitResultRelInfo(leaf_part_rri,
partrel,
1, /* dummy */
- false,
rel,
0);
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 70ecf108a3..4fed4e101f 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -189,7 +189,6 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
Relation partition_root,
int instrument_options);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index b120954997..6a6aac5a88 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -339,6 +339,12 @@ alter table p add constraint check_b check (b = 3);
insert into p values (1, 2);
ERROR: new row for relation "p11" violates check constraint "check_b"
DETAIL: Failing row contains (1, 2).
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+ERROR: new row for relation "p11" violates partition constraint
+DETAIL: Failing row contains (3, 2).
-- cleanup
drop table p cascade;
NOTICE: drop cascades to 2 other objects
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 3d2fdb92c5..171196df6d 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -200,5 +200,10 @@ alter table p add constraint check_b check (b = 3);
-- after "(1, 2)" is routed to it
insert into p values (1, 2);
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+
-- cleanup
drop table p cascade;
--
2.11.0
--------------C187C3B1305137A7AECB8485
Content-Type: text/x-diff;
name="0005-Add-some-more-tests-for-tuple-routing.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="0005-Add-some-more-tests-for-tuple-routing.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 4/7] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07 amit <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: amit @ 2016-12-13 06:07 UTC (permalink / raw)
Since implicit partition constraints are not inherited, an internal
partition's constraint was not being enforced when targeted directly.
So, include such constraint when setting up leaf partition result
relations for tuple-routing.
InitResultRelInfo()'s API changes with this. Instead of passing
a boolean telling whether or not to load the partition constraint,
callers now need to pass the exact constraint expression to use
as ri_PartitionCheck or NIL.
Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
src/backend/commands/copy.c | 7 +++++-
src/backend/commands/tablecmds.c | 2 +-
src/backend/executor/execMain.c | 42 ++++++++++++++++++++++++++++++------
src/include/executor/executor.h | 3 +--
src/test/regress/expected/insert.out | 4 ++++
src/test/regress/sql/insert.sql | 3 +++
6 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index d5901651db..a0eb4241e2 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2294,6 +2294,7 @@ CopyFrom(CopyState cstate)
uint64 processed = 0;
bool useHeapMultiInsert;
int nBufferedTuples = 0;
+ List *partcheck = NIL;
#define MAX_BUFFERED_TUPLES 1000
HeapTuple *bufferedTuples = NULL; /* initialize to silence warning */
@@ -2410,6 +2411,10 @@ CopyFrom(CopyState cstate)
hi_options |= HEAP_INSERT_FROZEN;
}
+ /* Don't forget the partition constraints */
+ if (cstate->rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(cstate->rel);
+
/*
* We need a ResultRelInfo so we can use the regular executor's
* index-entry-making machinery. (There used to be a huge amount of code
@@ -2419,7 +2424,7 @@ CopyFrom(CopyState cstate)
InitResultRelInfo(resultRelInfo,
cstate->rel,
1, /* dummy rangetable index */
- true, /* do load partition check expression */
+ partcheck,
0);
ExecOpenIndices(resultRelInfo, false);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d2e4cfc365..4bd4ec4e8c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1323,7 +1323,7 @@ ExecuteTruncate(TruncateStmt *stmt)
InitResultRelInfo(resultRelInfo,
rel,
0, /* dummy rangetable index */
- false,
+ NIL,
0);
resultRelInfo++;
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index b3cedadce4..1b19bc38ef 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -821,13 +821,19 @@ InitPlan(QueryDesc *queryDesc, int eflags)
Index resultRelationIndex = lfirst_int(l);
Oid resultRelationOid;
Relation resultRelation;
+ List *partcheck = NIL;
resultRelationOid = getrelid(resultRelationIndex, rangeTable);
resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+
+ /* Don't forget the partition constraint */
+ if (resultRelation->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(resultRelation);
+
InitResultRelInfo(resultRelInfo,
resultRelation,
resultRelationIndex,
- true,
+ partcheck,
estate->es_instrument);
resultRelInfo++;
}
@@ -1217,7 +1223,7 @@ void
InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
+ List *partition_check,
int instrument_options)
{
MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
@@ -1255,9 +1261,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
- if (load_partition_check)
- resultRelInfo->ri_PartitionCheck =
- RelationGetPartitionQual(resultRelationDesc);
+ resultRelInfo->ri_PartitionCheck = partition_check;
}
/*
@@ -1284,6 +1288,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
ListCell *l;
Relation rel;
MemoryContext oldcontext;
+ List *partcheck = NIL;
/* First, search through the query result relations */
rInfo = estate->es_result_relations;
@@ -1312,6 +1317,10 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
*/
rel = heap_open(relid, NoLock);
+ /* Don't forget the partition constraint */
+ if (rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(rel);
+
/*
* Make the new entry in the right context.
*/
@@ -1320,7 +1329,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
- true,
+ partcheck,
estate->es_instrument);
estate->es_trig_target_relations =
lappend(estate->es_trig_target_relations, rInfo);
@@ -3032,6 +3041,7 @@ ExecSetupPartitionTupleRouting(Relation rel,
ListCell *cell;
int i;
ResultRelInfo *leaf_part_rri;
+ List *partcheck = NIL;
/* Get the tuple-routing information and lock partitions */
*pd = RelationGetPartitionDispatchInfo(rel, RowExclusiveLock, num_parted,
@@ -3042,12 +3052,22 @@ ExecSetupPartitionTupleRouting(Relation rel,
*tup_conv_maps = (TupleConversionMap **) palloc0(*num_partitions *
sizeof(TupleConversionMap *));
+ /*
+ * If the main target rel is a partition, ExecConstraints() as applied to
+ * each leaf partition must consider its partition constraint, because
+ * unlike explicit constraints, an implicit partition constraint is not
+ * inherited.
+ */
+ if (rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(rel);
+
leaf_part_rri = *partitions;
i = 0;
foreach(cell, leaf_parts)
{
Relation partrel;
TupleDesc part_tupdesc;
+ List *my_check = NIL;
/*
* We locked all the partitions above including the leaf partitions.
@@ -3069,10 +3089,18 @@ ExecSetupPartitionTupleRouting(Relation rel,
(*tup_conv_maps)[i] = convert_tuples_by_name(tupDesc, part_tupdesc,
gettext_noop("could not convert row type"));
+ /*
+ * This is the parent's partition constraint, so any Vars in
+ * it bear the its attribute numbers. We must switch them to
+ * the leaf partition's.
+ */
+ if (partcheck)
+ my_check = map_partition_varattnos(partcheck, partrel, rel);
+
InitResultRelInfo(leaf_part_rri,
partrel,
1, /* dummy */
- false,
+ my_check,
0);
/* Open partition indices */
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index b74fa5eb5d..c8e42ae2eb 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -73,7 +73,6 @@
#define ExecEvalExpr(expr, econtext, isNull, isDone) \
((*(expr)->evalfunc) (expr, econtext, isNull, isDone))
-
/* Hook for plugins to get control in ExecutorStart() */
typedef void (*ExecutorStart_hook_type) (QueryDesc *queryDesc, int eflags);
extern PGDLLIMPORT ExecutorStart_hook_type ExecutorStart_hook;
@@ -189,7 +188,7 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
+ List *partition_check,
int instrument_options);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 561cefa3c4..95a7c4da7a 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -285,6 +285,10 @@ select tableoid::regclass, * from list_parted;
part_ee_ff2 | EE | 10
(8 rows)
+-- fail due to partition constraint failure
+insert into part_ee_ff values ('gg', 1);
+ERROR: new row for relation "part_ee_ff1" violates partition constraint
+DETAIL: Failing row contains (gg, 1).
-- cleanup
drop table range_parted cascade;
NOTICE: drop cascades to 4 other objects
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 846bb5897a..77577682ac 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -167,6 +167,9 @@ insert into list_parted values ('EE', 1);
insert into part_ee_ff values ('EE', 10);
select tableoid::regclass, * from list_parted;
+-- fail due to partition constraint failure
+insert into part_ee_ff values ('gg', 1);
+
-- cleanup
drop table range_parted cascade;
drop table list_parted cascade;
--
2.11.0
--------------6BBEDC11B506E66A7D75776C
Content-Type: text/x-diff;
name="0005-Fix-oddities-of-tuple-routing-and-TupleTableSlots-2.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0005-Fix-oddities-of-tuple-routing-and-TupleTableSlots-2.pat";
filename*1="ch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 4/5] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07 amit <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: amit @ 2016-12-13 06:07 UTC (permalink / raw)
Since implicit partition constraints are not inherited, an internal
partition's constraint was not being enforced when targeted directly.
So, include such constraint when setting up leaf partition result
relations for tuple-routing.
Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
src/backend/commands/copy.c | 1 -
src/backend/commands/tablecmds.c | 1 -
src/backend/executor/execMain.c | 41 ++++++++++++++++++++++++++++--------
src/include/executor/executor.h | 1 -
src/test/regress/expected/insert.out | 6 ++++++
src/test/regress/sql/insert.sql | 5 +++++
6 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index a20e22daaf..2cb89190e7 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2419,7 +2419,6 @@ CopyFrom(CopyState cstate)
InitResultRelInfo(resultRelInfo,
cstate->rel,
1, /* dummy rangetable index */
- true, /* do load partition check expression */
NULL,
0);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3db830f0c7..096ca181be 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1323,7 +1323,6 @@ ExecuteTruncate(TruncateStmt *stmt)
InitResultRelInfo(resultRelInfo,
rel,
0, /* dummy rangetable index */
- false,
NULL,
0);
resultRelInfo++;
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6954ba8d32..3e2c1cb790 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -824,10 +824,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
resultRelationOid = getrelid(resultRelationIndex, rangeTable);
resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+
InitResultRelInfo(resultRelInfo,
resultRelation,
resultRelationIndex,
- true,
NULL,
estate->es_instrument);
resultRelInfo++;
@@ -1218,10 +1218,11 @@ void
InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
Relation partition_root,
int instrument_options)
{
+ List *partition_check = NIL;
+
MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
resultRelInfo->type = T_ResultRelInfo;
resultRelInfo->ri_RangeTableIndex = resultRelationIndex;
@@ -1257,14 +1258,38 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
- if (load_partition_check)
- resultRelInfo->ri_PartitionCheck =
- RelationGetPartitionQual(resultRelationDesc);
/*
- * The following gets set to NULL unless we are initializing leaf
- * partitions for tuple-routing.
+ * If partition_root has been specified, that means we are builiding the
+ * ResultRelationInfo for one of its leaf partitions. In that case, we
+ * need *not* initialize the leaf partition's constraint, but rather the
+ * the partition_root's (if any). We must do that explicitly like this,
+ * because implicit partition constraints are not inherited like user-
+ * defined constraints and would fail to be enforced by ExecConstraints()
+ * after a tuple is routed to a leaf partition.
*/
+ if (partition_root)
+ {
+ /*
+ * Root table itself may or may not be a partition; partition_check
+ * would be NIL in the latter case.
+ */
+ partition_check = RelationGetPartitionQual(partition_root);
+
+ /*
+ * This is not our own partition constraint, but rather an ancestor's.
+ * So any Vars in it bear the ancestor's attribute numbers. We must
+ * switch them to our own.
+ */
+ if (partition_check != NIL)
+ partition_check = map_partition_varattnos(partition_check,
+ resultRelationDesc,
+ partition_root);
+ }
+ else
+ partition_check = RelationGetPartitionQual(resultRelationDesc);
+
+ resultRelInfo->ri_PartitionCheck = partition_check;
resultRelInfo->ri_PartitionRoot = partition_root;
}
@@ -1328,7 +1353,6 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
- true,
NULL,
estate->es_instrument);
estate->es_trig_target_relations =
@@ -3137,7 +3161,6 @@ ExecSetupPartitionTupleRouting(Relation rel,
InitResultRelInfo(leaf_part_rri,
partrel,
1, /* dummy */
- false,
rel,
0);
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index f85b26b00a..9c7878f847 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -189,7 +189,6 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
Relation partition_root,
int instrument_options);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index b120954997..6a6aac5a88 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -339,6 +339,12 @@ alter table p add constraint check_b check (b = 3);
insert into p values (1, 2);
ERROR: new row for relation "p11" violates check constraint "check_b"
DETAIL: Failing row contains (1, 2).
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+ERROR: new row for relation "p11" violates partition constraint
+DETAIL: Failing row contains (3, 2).
-- cleanup
drop table p cascade;
NOTICE: drop cascades to 2 other objects
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 3d2fdb92c5..171196df6d 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -200,5 +200,10 @@ alter table p add constraint check_b check (b = 3);
-- after "(1, 2)" is routed to it
insert into p values (1, 2);
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+
-- cleanup
drop table p cascade;
--
2.11.0
--------------57363B45666F3B09901F4C0C
Content-Type: text/x-diff;
name="0005-Add-some-more-tests-for-tuple-routing.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="0005-Add-some-more-tests-for-tuple-routing.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 4/7] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07 amit <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: amit @ 2016-12-13 06:07 UTC (permalink / raw)
Since implicit partition constraints are not inherited, an internal
partition's constraint was not being enforced when targeted directly.
So, include such constraint when setting up leaf partition result
relations for tuple-routing.
InitResultRelInfo()'s API changes with this. Instead of passing
a boolean telling whether or not to load the partition constraint,
callers now need to pass the exact constraint expression to use
as ri_PartitionCheck or NIL.
---
src/backend/commands/copy.c | 7 +++++-
src/backend/commands/tablecmds.c | 2 +-
src/backend/executor/execMain.c | 43 ++++++++++++++++++++++++++++++------
src/include/executor/executor.h | 3 +--
src/test/regress/expected/insert.out | 4 ++++
src/test/regress/sql/insert.sql | 3 +++
6 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index d5901651db..a0eb4241e2 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2294,6 +2294,7 @@ CopyFrom(CopyState cstate)
uint64 processed = 0;
bool useHeapMultiInsert;
int nBufferedTuples = 0;
+ List *partcheck = NIL;
#define MAX_BUFFERED_TUPLES 1000
HeapTuple *bufferedTuples = NULL; /* initialize to silence warning */
@@ -2410,6 +2411,10 @@ CopyFrom(CopyState cstate)
hi_options |= HEAP_INSERT_FROZEN;
}
+ /* Don't forget the partition constraints */
+ if (cstate->rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(cstate->rel);
+
/*
* We need a ResultRelInfo so we can use the regular executor's
* index-entry-making machinery. (There used to be a huge amount of code
@@ -2419,7 +2424,7 @@ CopyFrom(CopyState cstate)
InitResultRelInfo(resultRelInfo,
cstate->rel,
1, /* dummy rangetable index */
- true, /* do load partition check expression */
+ partcheck,
0);
ExecOpenIndices(resultRelInfo, false);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d2e4cfc365..4bd4ec4e8c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1323,7 +1323,7 @@ ExecuteTruncate(TruncateStmt *stmt)
InitResultRelInfo(resultRelInfo,
rel,
0, /* dummy rangetable index */
- false,
+ NIL,
0);
resultRelInfo++;
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index b3cedadce4..5627378a35 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -821,13 +821,19 @@ InitPlan(QueryDesc *queryDesc, int eflags)
Index resultRelationIndex = lfirst_int(l);
Oid resultRelationOid;
Relation resultRelation;
+ List *partcheck = NIL;
resultRelationOid = getrelid(resultRelationIndex, rangeTable);
resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+
+ /* Don't forget the partition constraint */
+ if (resultRelation->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(resultRelation);
+
InitResultRelInfo(resultRelInfo,
resultRelation,
resultRelationIndex,
- true,
+ partcheck,
estate->es_instrument);
resultRelInfo++;
}
@@ -1217,7 +1223,7 @@ void
InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
+ List *partition_check,
int instrument_options)
{
MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
@@ -1255,9 +1261,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
- if (load_partition_check)
- resultRelInfo->ri_PartitionCheck =
- RelationGetPartitionQual(resultRelationDesc);
+ resultRelInfo->ri_PartitionCheck = partition_check;
}
/*
@@ -1284,6 +1288,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
ListCell *l;
Relation rel;
MemoryContext oldcontext;
+ List *partcheck = NIL;
/* First, search through the query result relations */
rInfo = estate->es_result_relations;
@@ -1312,6 +1317,10 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
*/
rel = heap_open(relid, NoLock);
+ /* Don't forget the partition constraint */
+ if (rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(rel);
+
/*
* Make the new entry in the right context.
*/
@@ -1320,7 +1329,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
- true,
+ partcheck,
estate->es_instrument);
estate->es_trig_target_relations =
lappend(estate->es_trig_target_relations, rInfo);
@@ -3032,6 +3041,7 @@ ExecSetupPartitionTupleRouting(Relation rel,
ListCell *cell;
int i;
ResultRelInfo *leaf_part_rri;
+ List *partcheck = NIL;
/* Get the tuple-routing information and lock partitions */
*pd = RelationGetPartitionDispatchInfo(rel, RowExclusiveLock, num_parted,
@@ -3042,12 +3052,22 @@ ExecSetupPartitionTupleRouting(Relation rel,
*tup_conv_maps = (TupleConversionMap **) palloc0(*num_partitions *
sizeof(TupleConversionMap *));
+ /*
+ * If the main target rel is a partition, ExecConstraints() as applied to
+ * each leaf partition must consider its partition constraint, because
+ * unlike explicit constraints, an implicit partition constraint is not
+ * inherited.
+ */
+ if (rel->rd_rel->relispartition)
+ partcheck = RelationGetPartitionQual(rel);
+
leaf_part_rri = *partitions;
i = 0;
foreach(cell, leaf_parts)
{
Relation partrel;
TupleDesc part_tupdesc;
+ List *my_check = NIL;
/*
* We locked all the partitions above including the leaf partitions.
@@ -3069,10 +3089,19 @@ ExecSetupPartitionTupleRouting(Relation rel,
(*tup_conv_maps)[i] = convert_tuples_by_name(tupDesc, part_tupdesc,
gettext_noop("could not convert row type"));
+ /*
+ * This is the parent's partition constraint, so any Vars in
+ * it bear the its attribute numbers. We must switch them to
+ * the leaf partition's, which is possible with the
+ * reverse_map's attribute-number map.
+ */
+ if (partcheck)
+ my_check = map_partition_varattnos(partcheck, partrel, rel);
+
InitResultRelInfo(leaf_part_rri,
partrel,
1, /* dummy */
- false,
+ my_check,
0);
/* Open partition indices */
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index bd1bc6bb6e..8bcc876809 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -73,7 +73,6 @@
#define ExecEvalExpr(expr, econtext, isNull, isDone) \
((*(expr)->evalfunc) (expr, econtext, isNull, isDone))
-
/* Hook for plugins to get control in ExecutorStart() */
typedef void (*ExecutorStart_hook_type) (QueryDesc *queryDesc, int eflags);
extern PGDLLIMPORT ExecutorStart_hook_type ExecutorStart_hook;
@@ -189,7 +188,7 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
+ List *partition_check,
int instrument_options);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 561cefa3c4..95a7c4da7a 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -285,6 +285,10 @@ select tableoid::regclass, * from list_parted;
part_ee_ff2 | EE | 10
(8 rows)
+-- fail due to partition constraint failure
+insert into part_ee_ff values ('gg', 1);
+ERROR: new row for relation "part_ee_ff1" violates partition constraint
+DETAIL: Failing row contains (gg, 1).
-- cleanup
drop table range_parted cascade;
NOTICE: drop cascades to 4 other objects
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 846bb5897a..77577682ac 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -167,6 +167,9 @@ insert into list_parted values ('EE', 1);
insert into part_ee_ff values ('EE', 10);
select tableoid::regclass, * from list_parted;
+-- fail due to partition constraint failure
+insert into part_ee_ff values ('gg', 1);
+
-- cleanup
drop table range_parted cascade;
drop table list_parted cascade;
--
2.11.0
--------------487C465F02616E3296F6B86F
Content-Type: text/x-diff;
name="0005-Fix-oddities-of-tuple-routing-and-TupleTableSlots.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0005-Fix-oddities-of-tuple-routing-and-TupleTableSlots.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v28 2/5] Add pg_depend.refobjversion.
@ 2019-05-28 18:16 Thomas Munro <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Munro @ 2019-05-28 18:16 UTC (permalink / raw)
Provide a place for the version of referenced database objects to be
recorded. Later commits will be able to use this to record dependencies
on collation versions for indexes and maybe more.
Author: Thomas Munro <[email protected]>
Reviewed-by: Julien Rouhaud <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
---
doc/src/sgml/catalogs.sgml | 12 +++++++
src/backend/catalog/dependency.c | 14 +++++---
src/backend/catalog/pg_depend.c | 14 +++++---
src/bin/initdb/initdb.c | 44 +++++++++++------------
src/include/catalog/dependency.h | 1 +
src/include/catalog/pg_depend.h | 4 +++
src/include/catalog/toasting.h | 1 +
src/test/regress/expected/misc_sanity.out | 4 +--
8 files changed, 62 insertions(+), 32 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index a28ae89e13..2567863dd9 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -3293,6 +3293,18 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
A code defining the specific semantics of this dependency relationship; see text
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>refobjversion</structfield> <type>text</type>
+ </para>
+ <para>
+ An optional version for the referenced object. The only current use of
+ <structfield>refobjversion</structfield> is to record dependencies
+ between indexes and collation versions.
+ </para></entry>
+ </row>
+
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index f515e2c308..1a927377e7 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1600,7 +1600,9 @@ recordDependencyOnExpr(const ObjectAddress *depender,
/* And record 'em */
recordMultipleDependencies(depender,
- context.addrs->refs, context.addrs->numrefs,
+ context.addrs->refs,
+ context.addrs->numrefs,
+ NULL,
behavior);
free_object_addresses(context.addrs);
@@ -1687,7 +1689,9 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
/* Record the self-dependencies with the appropriate direction */
if (!reverse_self)
recordMultipleDependencies(depender,
- self_addrs->refs, self_addrs->numrefs,
+ self_addrs->refs,
+ self_addrs->numrefs,
+ NULL,
self_behavior);
else
{
@@ -1707,7 +1711,9 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
/* Record the external dependencies */
recordMultipleDependencies(depender,
- context.addrs->refs, context.addrs->numrefs,
+ context.addrs->refs,
+ context.addrs->numrefs,
+ NULL,
behavior);
free_object_addresses(context.addrs);
@@ -2679,7 +2685,7 @@ record_object_address_dependencies(const ObjectAddress *depender,
{
eliminate_duplicate_dependencies(referenced);
recordMultipleDependencies(depender,
- referenced->refs, referenced->numrefs,
+ referenced->refs, referenced->numrefs, NULL,
behavior);
}
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 70baf03178..d86f85d142 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -24,6 +24,7 @@
#include "catalog/pg_extension.h"
#include "commands/extension.h"
#include "miscadmin.h"
+#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/rel.h"
@@ -44,7 +45,7 @@ recordDependencyOn(const ObjectAddress *depender,
const ObjectAddress *referenced,
DependencyType behavior)
{
- recordMultipleDependencies(depender, referenced, 1, behavior);
+ recordMultipleDependencies(depender, referenced, 1, NULL, behavior);
}
/*
@@ -55,6 +56,7 @@ void
recordMultipleDependencies(const ObjectAddress *depender,
const ObjectAddress *referenced,
int nreferenced,
+ const char *version,
DependencyType behavior)
{
Relation dependDesc;
@@ -79,8 +81,6 @@ recordMultipleDependencies(const ObjectAddress *depender,
/* Don't open indexes unless we need to make an update */
indstate = NULL;
- memset(nulls, false, sizeof(nulls));
-
for (i = 0; i < nreferenced; i++, referenced++)
{
/*
@@ -94,6 +94,9 @@ recordMultipleDependencies(const ObjectAddress *depender,
* Record the Dependency. Note we don't bother to check for
* duplicate dependencies; there's no harm in them.
*/
+ memset(nulls, false, sizeof(nulls));
+ memset(values, 0, sizeof(values));
+
values[Anum_pg_depend_classid - 1] = ObjectIdGetDatum(depender->classId);
values[Anum_pg_depend_objid - 1] = ObjectIdGetDatum(depender->objectId);
values[Anum_pg_depend_objsubid - 1] = Int32GetDatum(depender->objectSubId);
@@ -101,9 +104,12 @@ recordMultipleDependencies(const ObjectAddress *depender,
values[Anum_pg_depend_refclassid - 1] = ObjectIdGetDatum(referenced->classId);
values[Anum_pg_depend_refobjid - 1] = ObjectIdGetDatum(referenced->objectId);
values[Anum_pg_depend_refobjsubid - 1] = Int32GetDatum(referenced->objectSubId);
+ if (version)
+ values[Anum_pg_depend_refobjversion - 1] = CStringGetTextDatum(version);
+ else
+ nulls[Anum_pg_depend_refobjversion - 1] = true;
values[Anum_pg_depend_deptype - 1] = CharGetDatum((char) behavior);
-
tup = heap_form_tuple(dependDesc->rd_att, values, nulls);
/* fetch index info only when we know we need it */
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 786672b1b6..4b2a58cf18 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1572,55 +1572,55 @@ setup_depend(FILE *cmdfd)
"DELETE FROM pg_shdepend;\n\n",
"VACUUM pg_shdepend;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_class;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_proc;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_type;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_cast;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_constraint;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_conversion;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_attrdef;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_language;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_operator;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_opclass;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_opfamily;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_am;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_amop;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_amproc;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_rewrite;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_trigger;\n\n",
/*
* restriction here to avoid pinning the public namespace
*/
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_namespace "
" WHERE nspname LIKE 'pg%';\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_ts_parser;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_ts_dict;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_ts_template;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_ts_config;\n\n",
- "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+ "INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',NULL"
" FROM pg_collation;\n\n",
"INSERT INTO pg_shdepend SELECT 0,0,0,0, tableoid,oid, 'p' "
" FROM pg_authid;\n\n",
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index a8f7e9965b..3baa5e498a 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -189,6 +189,7 @@ extern void recordDependencyOn(const ObjectAddress *depender,
extern void recordMultipleDependencies(const ObjectAddress *depender,
const ObjectAddress *referenced,
int nreferenced,
+ const char *version,
DependencyType behavior);
extern void recordDependencyOnCurrentExtension(const ObjectAddress *object,
diff --git a/src/include/catalog/pg_depend.h b/src/include/catalog/pg_depend.h
index ccf0a98330..7489022795 100644
--- a/src/include/catalog/pg_depend.h
+++ b/src/include/catalog/pg_depend.h
@@ -61,6 +61,10 @@ CATALOG(pg_depend,2608,DependRelationId)
* field. See DependencyType in catalog/dependency.h.
*/
char deptype; /* see codes in dependency.h */
+#ifdef CATALOG_VARLEN
+ text refobjversion; /* version tracking, NULL if not used or
+ * unknown */
+#endif
} FormData_pg_depend;
/* ----------------
diff --git a/src/include/catalog/toasting.h b/src/include/catalog/toasting.h
index 8f131893dc..e320d82203 100644
--- a/src/include/catalog/toasting.h
+++ b/src/include/catalog/toasting.h
@@ -53,6 +53,7 @@ DECLARE_TOAST(pg_aggregate, 4159, 4160);
DECLARE_TOAST(pg_attrdef, 2830, 2831);
DECLARE_TOAST(pg_constraint, 2832, 2833);
DECLARE_TOAST(pg_default_acl, 4143, 4144);
+DECLARE_TOAST(pg_depend, 8888, 8889);
DECLARE_TOAST(pg_description, 2834, 2835);
DECLARE_TOAST(pg_event_trigger, 4145, 4146);
DECLARE_TOAST(pg_extension, 4147, 4148);
diff --git a/src/test/regress/expected/misc_sanity.out b/src/test/regress/expected/misc_sanity.out
index 8538173ff8..d40afeef78 100644
--- a/src/test/regress/expected/misc_sanity.out
+++ b/src/test/regress/expected/misc_sanity.out
@@ -18,8 +18,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR
deptype NOT IN ('a', 'e', 'i', 'n', 'p') OR
(deptype != 'p' AND (classid = 0 OR objid = 0)) OR
(deptype = 'p' AND (classid != 0 OR objid != 0 OR objsubid != 0));
- classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype
----------+-------+----------+------------+----------+-------------+---------
+ classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype | refobjversion
+---------+-------+----------+------------+----------+-------------+---------+---------------
(0 rows)
-- **************** pg_shdepend ****************
--
2.20.1
--gclmslubawp3yaq7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename="v28-0003-Track-collation-versions-for-indexes.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: make BuiltinTrancheNames less ugly
@ 2024-02-23 09:31 Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Heikki Linnakangas @ 2024-02-23 09:31 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; Alvaro Herrera <[email protected]>; +Cc: Pg Hackers <[email protected]>
On 12/02/2024 19:01, Tristan Partin wrote:
> On Wed Jan 24, 2024 at 8:09 AM CST, Alvaro Herrera wrote:
>> IMO it would be less ugly to have the origin file lwlocknames.txt be
>> not a text file but a .h with a macro that can be defined by
>> interested parties so that they can extract what they want from the
>> file, like PG_CMDTAG or PG_KEYWORD. Using Perl for this seems dirty...
>
> I really like this idea, and would definitely be more inclined to see
> a solution like this.
+1 to that idea from me too. Seems pretty straightforward.
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: make BuiltinTrancheNames less ugly
@ 2024-03-01 14:00 Alvaro Herrera <[email protected]>
parent: Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Alvaro Herrera @ 2024-03-01 14:00 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Tristan Partin <[email protected]>; Pg Hackers <[email protected]>
On 2024-Feb-23, Heikki Linnakangas wrote:
> On 12/02/2024 19:01, Tristan Partin wrote:
> > On Wed Jan 24, 2024 at 8:09 AM CST, Alvaro Herrera wrote:
> > > IMO it would be less ugly to have the origin file lwlocknames.txt be
> > > not a text file but a .h with a macro that can be defined by
> > > interested parties so that they can extract what they want from the
> > > file, like PG_CMDTAG or PG_KEYWORD. Using Perl for this seems dirty...
> >
> > I really like this idea, and would definitely be more inclined to see
> > a solution like this.
>
> +1 to that idea from me too. Seems pretty straightforward.
OK, here's a patch that does it. I have not touched Meson yet.
I'm pretty disappointed of not being able to remove
generate-lwlocknames.pl (it now generates the .h, no longer the .c
file), but I can't find a way to do the equivalent #defines in CPP ...
it'd require invoking the C preprocessor twice. Maybe an intermediate
.h file would solve the problem, but I have no idea how would that work
with Meson. I guess I'll do it in Make and let somebody suggest a Meson
way.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"El hombre nunca sabe de lo que es capaz hasta que lo intenta" (C. Dickens)
Attachments:
[text/x-diff] 0001-Rework-lwlocknames.txt-to-become-lwlocklist.h.patch (15.4K, ../../[email protected]/2-0001-Rework-lwlocknames.txt-to-become-lwlocklist.h.patch)
download | inline diff:
From c70ae4eff6ec8e8965207d6e0d6d8fcdcd91cf16 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <[email protected]>
Date: Fri, 1 Mar 2024 13:03:10 +0100
Subject: [PATCH] Rework lwlocknames.txt to become lwlocklist.h
This saves some code and some space in BuiltinTrancheNames, as foreseen
in commit 74a730631065.
---
src/backend/Makefile | 4 +-
src/backend/storage/lmgr/.gitignore | 1 -
src/backend/storage/lmgr/Makefile | 9 +-
.../storage/lmgr/generate-lwlocknames.pl | 52 ++++++------
src/backend/storage/lmgr/lwlock.c | 15 ++--
src/backend/storage/lmgr/lwlocknames.txt | 60 --------------
.../utils/activity/wait_event_names.txt | 8 +-
src/include/storage/lwlocklist.h | 82 +++++++++++++++++++
8 files changed, 124 insertions(+), 107 deletions(-)
delete mode 100644 src/backend/storage/lmgr/lwlocknames.txt
create mode 100644 src/include/storage/lwlocklist.h
diff --git a/src/backend/Makefile b/src/backend/Makefile
index d66e2a4b9f..34bb6393c4 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -110,8 +110,8 @@ $(top_builddir)/src/port/libpgport_srv.a: | submake-libpgport
parser/gram.h: parser/gram.y
$(MAKE) -C parser gram.h
-storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl storage/lmgr/lwlocknames.txt utils/activity/wait_event_names.txt
- $(MAKE) -C storage/lmgr lwlocknames.h lwlocknames.c
+storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl $(top_srcdir)/src/include/storage/lwlocklist.h utils/activity/wait_event_names.txt
+ $(MAKE) -C storage/lmgr lwlocknames.h
utils/activity/wait_event_types.h: utils/activity/generate-wait_event_types.pl utils/activity/wait_event_names.txt
$(MAKE) -C utils/activity wait_event_types.h pgstat_wait_event.c wait_event_funcs_data.c
diff --git a/src/backend/storage/lmgr/.gitignore b/src/backend/storage/lmgr/.gitignore
index dab4c3f580..8e5b734f15 100644
--- a/src/backend/storage/lmgr/.gitignore
+++ b/src/backend/storage/lmgr/.gitignore
@@ -1,3 +1,2 @@
-/lwlocknames.c
/lwlocknames.h
/s_lock_test
diff --git a/src/backend/storage/lmgr/Makefile b/src/backend/storage/lmgr/Makefile
index 1aef423384..9d7dbe5abd 100644
--- a/src/backend/storage/lmgr/Makefile
+++ b/src/backend/storage/lmgr/Makefile
@@ -18,7 +18,6 @@ OBJS = \
lmgr.o \
lock.o \
lwlock.o \
- lwlocknames.o \
predicate.o \
proc.o \
s_lock.o \
@@ -35,11 +34,7 @@ s_lock_test: s_lock.c $(top_builddir)/src/common/libpgcommon.a $(top_builddir)/s
$(TASPATH) -L $(top_builddir)/src/common -lpgcommon \
-L $(top_builddir)/src/port -lpgport -lm -o s_lock_test
-# see notes in src/backend/parser/Makefile
-lwlocknames.c: lwlocknames.h
- touch $@
-
-lwlocknames.h: $(top_srcdir)/src/backend/storage/lmgr/lwlocknames.txt $(top_srcdir)/src/backend/utils/activity/wait_event_names.txt generate-lwlocknames.pl
+lwlocknames.h: $(top_srcdir)/src/include/storage/lwlocklist.h $(top_srcdir)/src/backend/utils/activity/wait_event_names.txt generate-lwlocknames.pl
$(PERL) $(srcdir)/generate-lwlocknames.pl $^
check: s_lock_test
@@ -47,4 +42,4 @@ check: s_lock_test
clean:
rm -f s_lock_test
- rm -f lwlocknames.h lwlocknames.c
+ rm -f lwlocknames.h
diff --git a/src/backend/storage/lmgr/generate-lwlocknames.pl b/src/backend/storage/lmgr/generate-lwlocknames.pl
index 7b93ecf6c1..f46634a180 100644
--- a/src/backend/storage/lmgr/generate-lwlocknames.pl
+++ b/src/backend/storage/lmgr/generate-lwlocknames.pl
@@ -1,6 +1,6 @@
#!/usr/bin/perl
#
-# Generate lwlocknames.h and lwlocknames.c from lwlocknames.txt
+# Generate lwlocknames.h from lwlocklist.h
# Copyright (c) 2000-2024, PostgreSQL Global Development Group
use strict;
@@ -14,26 +14,22 @@ my $continue = "\n";
GetOptions('outdir:s' => \$output_path);
-open my $lwlocknames, '<', $ARGV[0] or die;
+open my $lwlocklist, '<', $ARGV[0] or die;
open my $wait_event_names, '<', $ARGV[1] or die;
# Include PID in suffix in case parallel make runs this multiple times.
my $htmp = "$output_path/lwlocknames.h.tmp$$";
-my $ctmp = "$output_path/lwlocknames.c.tmp$$";
open my $h, '>', $htmp or die "Could not open $htmp: $!";
-open my $c, '>', $ctmp or die "Could not open $ctmp: $!";
my $autogen =
- "/* autogenerated from src/backend/storage/lmgr/lwlocknames.txt, do not edit */\n";
+ "/* autogenerated from src/backend/storage/lmgr/lwlocklist.h, do not edit */\n";
print $h $autogen;
print $h "/* there is deliberately not an #ifndef LWLOCKNAMES_H here */\n\n";
-print $c $autogen, "\n";
-print $c "const char *const IndividualLWLockNames[] = {";
#
# First, record the predefined LWLocks listed in wait_event_names.txt. We'll
-# cross-check those with the ones in lwlocknames.txt.
+# cross-check those with the ones in lwlocklist.h.
#
my @wait_event_lwlocks;
my $record_lwlocks = 0;
@@ -64,17 +60,30 @@ while (<$wait_event_names>)
push(@wait_event_lwlocks, $waiteventname . "Lock");
}
+my $in_comment = 0;
my $i = 0;
-while (<$lwlocknames>)
+while (<$lwlocklist>)
{
chomp;
- # Skip comments
- next if /^#/;
+ # Skip uniline C comments and empty lines
+ next if m{^\s*/\*.*\*/$};
next if /^\s*$/;
- die "unable to parse lwlocknames.txt"
- unless /^(\w+)\s+(\d+)$/;
+ # skip multiline C comments
+ if ($in_comment == 1)
+ {
+ $in_comment = 0 if m{\*/};
+ next;
+ }
+ elsif (m{^\s*/\*})
+ {
+ $in_comment = 1;
+ next;
+ }
+
+ die "unable to parse lwlocklist.h line \"$_\""
+ unless /^PG_LWLOCK\((\w+),\s+(\d+)\)$/;
(my $lockname, my $lockidx) = ($1, $2);
@@ -82,25 +91,23 @@ while (<$lwlocknames>)
$trimmedlockname =~ s/Lock$//;
die "lock names must end with 'Lock'" if $trimmedlockname eq $lockname;
- die "lwlocknames.txt not in order" if $lockidx < $lastlockidx;
- die "lwlocknames.txt has duplicates" if $lockidx == $lastlockidx;
+ die "lwlocklist.h not in order" if $lockidx < $lastlockidx;
+ die "lwlocklist.h has duplicates" if $lockidx == $lastlockidx;
- die "$lockname defined in lwlocknames.txt but missing from "
+ die "$lockname defined in lwlocklist.h but missing from "
. "wait_event_names.txt"
if $i >= scalar @wait_event_lwlocks;
die "lists of predefined LWLocks do not match (first mismatch at "
. "$wait_event_lwlocks[$i] in wait_event_names.txt and $lockname in "
- . "lwlocknames.txt)"
+ . "lwlocklist.h)"
if $wait_event_lwlocks[$i] ne $lockname;
$i++;
while ($lastlockidx < $lockidx - 1)
{
++$lastlockidx;
- printf $c "%s \"<unassigned:%d>\"", $continue, $lastlockidx;
$continue = ",\n";
}
- printf $c "%s \"%s\"", $continue, $trimmedlockname;
$lastlockidx = $lockidx;
$continue = ",\n";
@@ -109,18 +116,15 @@ while (<$lwlocknames>)
die
"$wait_event_lwlocks[$i] defined in wait_event_names.txt but missing from "
- . "lwlocknames.txt"
+ . "lwlocklist.h"
if $i < scalar @wait_event_lwlocks;
-printf $c "\n};\n";
print $h "\n";
printf $h "#define NUM_INDIVIDUAL_LWLOCKS %s\n", $lastlockidx + 1;
close $h;
-close $c;
rename($htmp, "$output_path/lwlocknames.h")
|| die "rename: $htmp to $output_path/lwlocknames.h: $!";
-rename($ctmp, "$output_path/lwlocknames.c") || die "rename: $ctmp: $!";
-close $lwlocknames;
+close $lwlocklist;
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index d405c61b21..1112389af5 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -115,8 +115,8 @@ StaticAssertDecl(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS,
* There are three sorts of LWLock "tranches":
*
* 1. The individually-named locks defined in lwlocknames.h each have their
- * own tranche. The names of these tranches appear in IndividualLWLockNames[]
- * in lwlocknames.c.
+ * own tranche. We absorb the names of these tranches from there into
+ * BuiltinTrancheNames here.
*
* 2. There are some predefined tranches for built-in groups of locks.
* These are listed in enum BuiltinTrancheIds in lwlock.h, and their names
@@ -129,9 +129,10 @@ StaticAssertDecl(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS,
* All these names are user-visible as wait event names, so choose with care
* ... and do not forget to update the documentation's list of wait events.
*/
-extern const char *const IndividualLWLockNames[]; /* in lwlocknames.c */
-
static const char *const BuiltinTrancheNames[] = {
+#define PG_LWLOCK(lockname, id) [id] = CppAsString(lockname),
+#include "storage/lwlocknames.h"
+#undef PG_LWLOCK
[LWTRANCHE_XACT_BUFFER] = "XactBuffer",
[LWTRANCHE_COMMITTS_BUFFER] = "CommitTsBuffer",
[LWTRANCHE_SUBTRANS_BUFFER] = "SubtransBuffer",
@@ -745,11 +746,7 @@ LWLockReportWaitEnd(void)
static const char *
GetLWTrancheName(uint16 trancheId)
{
- /* Individual LWLock? */
- if (trancheId < NUM_INDIVIDUAL_LWLOCKS)
- return IndividualLWLockNames[trancheId];
-
- /* Built-in tranche? */
+ /* Built-in tranche or individual LWLock? */
if (trancheId < LWTRANCHE_FIRST_USER_DEFINED)
return BuiltinTrancheNames[trancheId];
diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txt
deleted file mode 100644
index 284d168f77..0000000000
--- a/src/backend/storage/lmgr/lwlocknames.txt
+++ /dev/null
@@ -1,60 +0,0 @@
-# Some commonly-used locks have predefined positions within MainLWLockArray;
-# these are defined here. If you add a lock, add it to the end to avoid
-# renumbering the existing locks; if you remove a lock, consider leaving a gap
-# in the numbering sequence for the benefit of DTrace and other external
-# debugging scripts. Also, do not forget to update the section
-# WaitEventLWLock of src/backend/utils/activity/wait_event_names.txt.
-
-# 0 is available; was formerly BufFreelistLock
-ShmemIndexLock 1
-OidGenLock 2
-XidGenLock 3
-ProcArrayLock 4
-SInvalReadLock 5
-SInvalWriteLock 6
-WALBufMappingLock 7
-WALWriteLock 8
-ControlFileLock 9
-# 10 was CheckpointLock
-# 11 was XactSLRULock
-# 12 was SubtransSLRULock
-MultiXactGenLock 13
-# 14 was MultiXactOffsetSLRULock
-# 15 was MultiXactMemberSLRULock
-RelCacheInitLock 16
-CheckpointerCommLock 17
-TwoPhaseStateLock 18
-TablespaceCreateLock 19
-BtreeVacuumLock 20
-AddinShmemInitLock 21
-AutovacuumLock 22
-AutovacuumScheduleLock 23
-SyncScanLock 24
-RelationMappingLock 25
-#26 was NotifySLRULock
-NotifyQueueLock 27
-SerializableXactHashLock 28
-SerializableFinishedListLock 29
-SerializablePredicateListLock 30
-# 31 was SerialSLRULock
-SyncRepLock 32
-BackgroundWorkerLock 33
-DynamicSharedMemoryControlLock 34
-AutoFileLock 35
-ReplicationSlotAllocationLock 36
-ReplicationSlotControlLock 37
-#38 was CommitTsSLRULock
-CommitTsLock 39
-ReplicationOriginLock 40
-MultiXactTruncationLock 41
-# 42 was OldSnapshotTimeMapLock
-LogicalRepWorkerLock 43
-XactTruncationLock 44
-# 45 was XactTruncationLock until removal of BackendRandomLock
-WrapLimitsVacuumLock 46
-NotifyQueueTailLock 47
-WaitEventExtensionLock 48
-WALSummarizerLock 49
-DSMRegistryLock 50
-InjectionPointLock 51
-SerialControlLock 52
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index ec2f31f82a..105ca977a5 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -279,9 +279,9 @@ Extension "Waiting in an extension."
# This class of wait events has its own set of C structure, so these are
# only used for the documentation.
#
-# NB: Predefined LWLocks (i.e., those declared in lwlocknames.txt) must be
+# NB: Predefined LWLocks (i.e., those declared in lwlocknames.h) must be
# listed in the top section of locks and must be listed in the same order as in
-# lwlocknames.txt.
+# lwlocknames.h.
#
Section: ClassName - WaitEventLWLock
@@ -332,9 +332,9 @@ SerialControl "Waiting to read or update shared <filename>pg_serial</filename> s
#
# END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE)
#
-# Predefined LWLocks (i.e., those declared in lwlocknames.txt) must be listed
+# Predefined LWLocks (i.e., those declared in lwlocknames.h) must be listed
# in the section above and must be listed in the same order as in
-# lwlocknames.txt. Other LWLocks must be listed in the section below.
+# lwlocknames.h. Other LWLocks must be listed in the section below.
#
XactBuffer "Waiting for I/O on a transaction status SLRU buffer."
diff --git a/src/include/storage/lwlocklist.h b/src/include/storage/lwlocklist.h
new file mode 100644
index 0000000000..3396eed58e
--- /dev/null
+++ b/src/include/storage/lwlocklist.h
@@ -0,0 +1,82 @@
+/*-------------------------------------------------------------------------
+ *
+ * lwlocklist.h
+ *
+ * The predefined LWLock list is kept in its own source file for use by
+ * automatic tools. The exact representation of a keyword is determined by
+ * the PG_LWLOCK macro, which is not defined in this file; it can be
+ * defined by the caller for special purposes.
+ *
+ * Also, generate-lwlock-names.pl processes this file.
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ * src/include/storage/lwlocklist.h
+ *
+ *-------------------------------------------------------------------------
+ */
+
+/*
+ * Some commonly-used locks have predefined positions within MainLWLockArray;
+ * these are defined here. If you add a lock, add it to the end to avoid
+ * renumbering the existing locks; if you remove a lock, consider leaving a gap
+ * in the numbering sequence for the benefit of DTrace and other external
+ * debugging scripts. Also, do not forget to update the section
+ * WaitEventLWLock of src/backend/utils/activity/wait_event_names.txt.
+ */
+
+/* 0 is available; was formerly BufFreelistLock */
+PG_LWLOCK(ShmemIndexLock, 1)
+PG_LWLOCK(OidGenLock, 2)
+PG_LWLOCK(XidGenLock, 3)
+PG_LWLOCK(ProcArrayLock, 4)
+PG_LWLOCK(SInvalReadLock, 5)
+PG_LWLOCK(SInvalWriteLock, 6)
+PG_LWLOCK(WALBufMappingLock, 7)
+PG_LWLOCK(WALWriteLock, 8)
+PG_LWLOCK(ControlFileLock, 9)
+/* 10 was CheckpointLock */
+/* 11 was XactSLRULock */
+/* 12 was SubtransSLRULock */
+PG_LWLOCK(MultiXactGenLock, 13)
+/* 14 was MultiXactOffsetSLRULock */
+/* 15 was MultiXactMemberSLRULock */
+PG_LWLOCK(RelCacheInitLock, 16)
+PG_LWLOCK(CheckpointerCommLock, 17)
+PG_LWLOCK(TwoPhaseStateLock, 18)
+PG_LWLOCK(TablespaceCreateLock, 19)
+PG_LWLOCK(BtreeVacuumLock, 20)
+PG_LWLOCK(AddinShmemInitLock, 21)
+PG_LWLOCK(AutovacuumLock, 22)
+PG_LWLOCK(AutovacuumScheduleLock, 23)
+PG_LWLOCK(SyncScanLock, 24)
+PG_LWLOCK(RelationMappingLock, 25)
+/* 26 was NotifySLRULock */
+PG_LWLOCK(NotifyQueueLock, 27)
+PG_LWLOCK(SerializableXactHashLock, 28)
+PG_LWLOCK(SerializableFinishedListLock, 29)
+PG_LWLOCK(SerializablePredicateListLock, 30)
+/* 31 was SerialSLRULock */
+PG_LWLOCK(SyncRepLock, 32)
+PG_LWLOCK(BackgroundWorkerLock, 33)
+PG_LWLOCK(DynamicSharedMemoryControlLock, 34)
+PG_LWLOCK(AutoFileLock, 35)
+PG_LWLOCK(ReplicationSlotAllocationLock, 36)
+PG_LWLOCK(ReplicationSlotControlLock, 37)
+/* 38 was CommitTsSLRULock */
+PG_LWLOCK(CommitTsLock, 39)
+PG_LWLOCK(ReplicationOriginLock, 40)
+PG_LWLOCK(MultiXactTruncationLock, 41)
+/* 42 was OldSnapshotTimeMapLock */
+PG_LWLOCK(LogicalRepWorkerLock, 43)
+PG_LWLOCK(XactTruncationLock, 44)
+/* 45 was XactTruncationLock until removal of BackendRandomLock */
+PG_LWLOCK(WrapLimitsVacuumLock, 46)
+PG_LWLOCK(NotifyQueueTailLock, 47)
+PG_LWLOCK(WaitEventExtensionLock, 48)
+PG_LWLOCK(WALSummarizerLock, 49)
+PG_LWLOCK(DSMRegistryLock, 50)
+PG_LWLOCK(InjectionPointLock, 51)
+PG_LWLOCK(SerialControlLock, 52)
--
2.39.2
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: make BuiltinTrancheNames less ugly
@ 2024-03-01 16:42 Tristan Partin <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Tristan Partin @ 2024-03-01 16:42 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; Heikki Linnakangas <[email protected]>; +Cc: Pg Hackers <[email protected]>
On Fri Mar 1, 2024 at 8:00 AM CST, Alvaro Herrera wrote:
> On 2024-Feb-23, Heikki Linnakangas wrote:
>
> > On 12/02/2024 19:01, Tristan Partin wrote:
> > > On Wed Jan 24, 2024 at 8:09 AM CST, Alvaro Herrera wrote:
> > > > IMO it would be less ugly to have the origin file lwlocknames.txt be
> > > > not a text file but a .h with a macro that can be defined by
> > > > interested parties so that they can extract what they want from the
> > > > file, like PG_CMDTAG or PG_KEYWORD. Using Perl for this seems dirty...
> > >
> > > I really like this idea, and would definitely be more inclined to see
> > > a solution like this.
> >
> > +1 to that idea from me too. Seems pretty straightforward.
>
> OK, here's a patch that does it. I have not touched Meson yet.
>
> I'm pretty disappointed of not being able to remove
> generate-lwlocknames.pl (it now generates the .h, no longer the .c
> file), but I can't find a way to do the equivalent #defines in CPP ...
> it'd require invoking the C preprocessor twice. Maybe an intermediate
> .h file would solve the problem, but I have no idea how would that work
> with Meson. I guess I'll do it in Make and let somebody suggest a Meson
> way.
I can help you with Meson if you get the Make implementation done.
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2024-03-01 16:42 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2016-12-13 06:07 [PATCH 4/7] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/7] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/7] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/5] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/7] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/5] Fix a bug of insertion into an internal partition. amit <[email protected]>
2019-05-28 18:16 [PATCH v28 2/5] Add pg_depend.refobjversion. Thomas Munro <[email protected]>
2024-02-23 09:31 Re: make BuiltinTrancheNames less ugly Heikki Linnakangas <[email protected]>
2024-03-01 14:00 ` Re: make BuiltinTrancheNames less ugly Alvaro Herrera <[email protected]>
2024-03-01 16:42 ` Re: make BuiltinTrancheNames less ugly Tristan Partin <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox