Hi,
I'm playing with partitioned tables and found a minor thing with the
error reporting of bounds checking when create partitions.
In function check_new_partition_bound(), there are three places where
we call ereport() with a parser_errposition(pstate, spec->location)
argument. However, that pstate is a dummy ParseState made from NULL,
so the error message never reports the position of the error in the
source query line.
I have attached a patch to pass in a ParseState to
check_new_partition_bound() to enable the reporting of the error
position. Below is what the error message looks like before and after
applying the patch.
-- Create parent table
create table foo (a int, b date) partition by range (b);
-- Before:
create table foo_part_1 partition of foo for values from (date
'2007-01-01') to (date '2006-01-01');
ERROR: empty range bound specified for partition "foo_part_1"
DETAIL: Specified lower bound ('2007-01-01') is greater than or equal to
upper bound ('2006-01-01').
-- After:
create table foo_part_1 partition of foo for values from (date
'2007-01-01') to (date '2006-01-01');
ERROR: empty range bound specified for partition "foo_part_1"
LINE 1: ...eate table foo_part_1 partition of foo for values from (date...
^
DETAIL: Specified lower bound ('2007-01-01') is greater than or equal to
upper bound ('2006-01-01').
Another option is to not pass the parser_errposition() argument at all
to ereport() in this function, since the query is relatively short and
the error message is already descriptive enough.
Alex and Ashwin
From b1aedddaf7ae0f609910ff82af136d93c0132c2e Mon Sep 17 00:00:00 2001
From: Alexandra Wang <lew...@pivotal.io>
Date: Wed, 8 Apr 2020 16:07:28 -0700
Subject: [PATCH] Report error position in partition bound check
We have been passing a dummy ParseState to ereport(). Without the source
text in the ParseState ereport does not report the error position even
if a error location is supplied. This patch passes a ParseState to
check_new_partition_bound() when it is available.
-- Create parent table
create table foo (a int, b date) partition by range (b);
-- Before:
create table foo_part_1 partition of foo for values from (date '2007-01-01') to (date '2006-01-01');
ERROR: empty range bound specified for partition "foo_part_1"
DETAIL: Specified lower bound ('2007-01-01') is greater than or equal to upper bound ('2006-01-01').
-- After:
create table foo_part_1 partition of foo for values from (date '2007-01-01') to (date '2006-01-01');
ERROR: empty range bound specified for partition "foo_part_1"
LINE 1: ...eate table foo_part_1 partition of foo for values from (date...
^
DETAIL: Specified lower bound ('2007-01-01') is greater than or equal to upper bound ('2006-01-01').
Co-authored-by: Ashwin Agrawal<aagra...@pivotal.io>
---
src/backend/commands/tablecmds.c | 4 ++--
src/backend/partitioning/partbounds.c | 3 +--
src/include/partitioning/partbounds.h | 4 +++-
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6162fb018c..46df40bee8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1004,7 +1004,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
* Check first that the new partition's bound is valid and does not
* overlap with any of existing partitions of the parent.
*/
- check_new_partition_bound(relname, parent, bound);
+ check_new_partition_bound(relname, parent, bound, pstate);
/*
* If the default partition exists, its partition constraints will
@@ -16268,7 +16268,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
* error.
*/
check_new_partition_bound(RelationGetRelationName(attachrel), rel,
- cmd->bound);
+ cmd->bound, NULL);
/* OK to create inheritance. Rest of the checks performed there */
CreateInheritance(attachrel, rel);
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 7607501fe7..dd56832efc 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -2803,12 +2803,11 @@ partitions_are_ordered(PartitionBoundInfo boundinfo, int nparts)
*/
void
check_new_partition_bound(char *relname, Relation parent,
- PartitionBoundSpec *spec)
+ PartitionBoundSpec *spec, ParseState *pstate)
{
PartitionKey key = RelationGetPartitionKey(parent);
PartitionDesc partdesc = RelationGetPartitionDesc(parent);
PartitionBoundInfo boundinfo = partdesc->boundinfo;
- ParseState *pstate = make_parsestate(NULL);
int with = -1;
bool overlap = false;
diff --git a/src/include/partitioning/partbounds.h b/src/include/partitioning/partbounds.h
index dfc720720b..c82f77d02f 100644
--- a/src/include/partitioning/partbounds.h
+++ b/src/include/partitioning/partbounds.h
@@ -14,6 +14,7 @@
#include "fmgr.h"
#include "nodes/parsenodes.h"
#include "nodes/pg_list.h"
+#include "parser/parse_node.h"
#include "partitioning/partdefs.h"
#include "utils/relcache.h"
struct RelOptInfo; /* avoid including pathnodes.h here */
@@ -98,7 +99,8 @@ extern PartitionBoundInfo partition_bounds_merge(int partnatts,
List **inner_parts);
extern bool partitions_are_ordered(PartitionBoundInfo boundinfo, int nparts);
extern void check_new_partition_bound(char *relname, Relation parent,
- PartitionBoundSpec *spec);
+ PartitionBoundSpec *spec,
+ ParseState *pstate);
extern void check_default_partition_contents(Relation parent,
Relation defaultRel,
PartitionBoundSpec *new_spec);
--
2.26.0
