Skip to content

Commit 7283aa4

Browse files
zilderzilder
committed
Merge branch 'rel_future_intervalcheck' into rel_future_beta
2 parents b4dd837 + b074c0f commit 7283aa4

File tree

4 files changed

+253
-14
lines changed

4 files changed

+253
-14
lines changed

‎Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ REGRESS = pathman_basic \
2626
pathman_permissions \
2727
pathman_rowmarks \
2828
pathman_utility_stmt_hooking \
29-
pathman_calamity
29+
pathman_calamity \
30+
pathman_interval
3031
EXTRA_REGRESS_OPTS=--temp-config=$(top_srcdir)/$(subdir)/conf.add
3132
EXTRA_CLEAN = pg_pathman--$(EXTVERSION).sql ./isolation_output
3233

‎expected/pathman_interval.out

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
\set VERBOSITY terse
2+
CREATE EXTENSION pg_pathman;
3+
/* Range partitions for INTEGER type */
4+
CREATE TABLE abc (id SERIAL);
5+
SELECT create_range_partitions('abc', 'id', 0, 100, 2);
6+
create_range_partitions
7+
-------------------------
8+
2
9+
(1 row)
10+
11+
SELECT set_interval('abc', NULL::INTEGER);
12+
set_interval
13+
--------------
14+
15+
(1 row)
16+
17+
/* pg_pathman shouldn't be able to create a new partition */
18+
INSERT INTO abc VALUES (250);
19+
ERROR: could not create new partitions for relation "abc"
20+
/* Set a trivial interval */
21+
SELECT set_interval('abc', 0);
22+
ERROR: interval must not be trivial
23+
/* We also shouldn't be able to set a trivial interval directly in pathman_config table */
24+
UPDATE pathman_config SET range_interval = '0' WHERE partrel = 'abc'::REGCLASS;
25+
ERROR: interval must not be trivial
26+
/* Set a normal interval */
27+
SELECT set_interval('abc', 1000);
28+
set_interval
29+
--------------
30+
31+
(1 row)
32+
33+
INSERT INTO abc VALUES (250);
34+
SELECT * FROM pathman_config;
35+
partrel | attname | parttype | range_interval
36+
---------+---------+----------+----------------
37+
abc | id | 2 | 1000
38+
(1 row)
39+
40+
DROP TABLE abc cascade;
41+
NOTICE: drop cascades to 3 other objects
42+
/* Range partitions for DATE type */
43+
CREATE TABLE abc (dt DATE NOT NULL);
44+
SELECT create_range_partitions('abc', 'dt', '2016-01-01'::DATE, '1 day'::INTERVAL, 2);
45+
create_range_partitions
46+
-------------------------
47+
2
48+
(1 row)
49+
50+
SELECT set_interval('abc', NULL::INTERVAL);
51+
set_interval
52+
--------------
53+
54+
(1 row)
55+
56+
/* Set a trivial interval */
57+
SELECT set_interval('abc', '1 second'::INTERVAL);
58+
ERROR: interval must not be trivial
59+
/* Set a normal interval */
60+
SELECT set_interval('abc', '1 month'::INTERVAL);
61+
set_interval
62+
--------------
63+
64+
(1 row)
65+
66+
SELECT * FROM pathman_config;
67+
partrel | attname | parttype | range_interval
68+
---------+---------+----------+----------------
69+
abc | dt | 2 | @ 1 mon
70+
(1 row)
71+
72+
DROP TABLE abc cascade;
73+
NOTICE: drop cascades to 2 other objects
74+
/* Hash partitioned table shouldn't accept any interval value */
75+
CREATE TABLE abc (id SERIAL);
76+
SELECT create_hash_partitions('abc', 'id', 3);
77+
create_hash_partitions
78+
------------------------
79+
3
80+
(1 row)
81+
82+
SELECT set_interval('abc', 100);
83+
ERROR: table "abc" is not partitioned by RANGE
84+
SELECT set_interval('abc', NULL::INTEGER);
85+
ERROR: table "abc" is not partitioned by RANGE
86+
DROP TABLE abc cascade;
87+
NOTICE: drop cascades to 3 other objects
88+
DROP EXTENSION pg_pathman;

‎sql/pathman_interval.sql

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
\set VERBOSITY terse
2+
CREATE EXTENSION pg_pathman;
3+
4+
/* Range partitions for INTEGER type */
5+
CREATE TABLE abc (id SERIAL);
6+
SELECT create_range_partitions('abc', 'id', 0, 100, 2);
7+
SELECT set_interval('abc', NULL::INTEGER);
8+
/* pg_pathman shouldn't be able to create a new partition */
9+
INSERT INTO abc VALUES (250);
10+
/* Set a trivial interval */
11+
SELECT set_interval('abc', 0);
12+
/* We also shouldn't be able to set a trivial interval directly in pathman_config table */
13+
UPDATE pathman_config SET range_interval = '0' WHERE partrel = 'abc'::REGCLASS;
14+
/* Set a normal interval */
15+
SELECT set_interval('abc', 1000);
16+
INSERT INTO abc VALUES (250);
17+
SELECT * FROM pathman_config;
18+
DROP TABLE abc cascade;
19+
20+
/* Range partitions for DATE type */
21+
CREATE TABLE abc (dt DATE NOT NULL);
22+
SELECT create_range_partitions('abc', 'dt', '2016-01-01'::DATE, '1 day'::INTERVAL, 2);
23+
SELECT set_interval('abc', NULL::INTERVAL);
24+
/* Set a trivial interval */
25+
SELECT set_interval('abc', '1 second'::INTERVAL);
26+
/* Set a normal interval */
27+
SELECT set_interval('abc', '1 month'::INTERVAL);
28+
SELECT * FROM pathman_config;
29+
DROP TABLE abc cascade;
30+
31+
/* Hash partitioned table shouldn't accept any interval value */
32+
CREATE TABLE abc (id SERIAL);
33+
SELECT create_hash_partitions('abc', 'id', 3);
34+
SELECT set_interval('abc', 100);
35+
SELECT set_interval('abc', NULL::INTEGER);
36+
DROP TABLE abc cascade;
37+
38+
DROP EXTENSION pg_pathman;

‎src/pl_range_funcs.c

Lines changed: 125 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
#include "utils/array.h"
2727
#include "utils/builtins.h"
2828
#include "utils/lsyscache.h"
29+
#include "utils/numeric.h"
2930
#include "utils/ruleutils.h"
31+
#include "utils/syscache.h"
3032

3133

3234
static char *deparse_constraint(Oid relid, Node *expr);
@@ -48,7 +50,9 @@ static void modify_range_constraint(Oid child_relid,
4850
const Bound *upper);
4951
static char *get_qualified_rel_name(Oid relid);
5052
static void drop_table_by_oid(Oid relid);
51-
53+
static bool interval_is_trivial(Oid atttype,
54+
Datum interval,
55+
Oid interval_type);
5256

5357
/* Function declarations */
5458

@@ -620,10 +624,9 @@ validate_interval_value(PG_FUNCTION_ARGS)
620624
Oid partrel = PG_GETARG_OID(0);
621625
text *attname = PG_GETARG_TEXT_P(1);
622626
PartType parttype = DatumGetPartType(PG_GETARG_DATUM(2));
623-
Datum range_interval = PG_GETARG_DATUM(3);
624-
625-
char *attname_cstr;
626-
Oid atttype; /* type of partitioned attribute */
627+
Datum interval_text = PG_GETARG_DATUM(3);
628+
Datum interval_value;
629+
Oid interval_type;
627630

628631
if (PG_ARGISNULL(0))
629632
elog(ERROR, "'partrel' should not be NULL");
@@ -634,21 +637,130 @@ validate_interval_value(PG_FUNCTION_ARGS)
634637
if (PG_ARGISNULL(2))
635638
elog(ERROR, "'parttype' should not be NULL");
636639

637-
/* it's OK if interval is NULL and table is HASH-partitioned */
638-
if (PG_ARGISNULL(3))
639-
PG_RETURN_BOOL(parttype == PT_HASH);
640+
/*
641+
* NULL interval is fine for both HASH and RANGE. But for RANGE we need
642+
* to make some additional checks
643+
*/
644+
if (!PG_ARGISNULL(3))
645+
{
646+
char *attname_cstr;
647+
Oid atttype; /* type of partitioned attribute */
648+
649+
if (parttype == PT_HASH)
650+
elog(ERROR, "interval must be NULL for HASH partitioned table");
640651

641-
/* Convert attname to CSTRING and fetch column's type */
642-
attname_cstr = text_to_cstring(attname);
643-
atttype = get_attribute_type(partrel, attname_cstr, false);
652+
/* Convert attname to CSTRING and fetch column's type */
653+
attname_cstr = text_to_cstring(attname);
654+
atttype = get_attribute_type(partrel, attname_cstr, false);
644655

645-
/* Try converting textual representation */
646-
extract_binary_interval_from_text(range_interval, atttype, NULL);
656+
/* Try converting textual representation */
657+
interval_value = extract_binary_interval_from_text(interval_text,
658+
atttype,
659+
&interval_type);
660+
661+
/* Check that interval isn't trivial */
662+
if (interval_is_trivial(atttype, interval_value, interval_type))
663+
elog(ERROR, "interval must not be trivial");
664+
}
647665

648666
PG_RETURN_BOOL(true);
649667
}
650668

651669

670+
/*
671+
* Check if interval is insignificant to avoid infinite loops while adding
672+
* new partitions
673+
*
674+
* The main idea behind this function is to add specified interval to some
675+
* default value (zero for numeric types and current date/timestamp for datetime
676+
* types) and look if it is changed. If it is then return true.
677+
*/
678+
static bool
679+
interval_is_trivial(Oid atttype, Datum interval, Oid interval_type)
680+
{
681+
Datum default_value;
682+
Datum op_result;
683+
Oid op_result_type;
684+
Operator op;
685+
Oid op_func;
686+
FmgrInfo cmp_func;
687+
688+
/* Generate default value */
689+
switch(atttype)
690+
{
691+
case INT2OID:
692+
case INT4OID:
693+
case INT8OID:
694+
default_value = Int16GetDatum(0);
695+
break;
696+
case FLOAT4OID:
697+
default_value = Float4GetDatum(0);
698+
break;
699+
case FLOAT8OID:
700+
default_value = Float8GetDatum(0);
701+
break;
702+
case NUMERICOID:
703+
default_value = NumericGetDatum(0);
704+
break;
705+
case TIMESTAMPOID:
706+
case TIMESTAMPTZOID:
707+
default_value = TimestampGetDatum(GetCurrentTimestamp());
708+
break;
709+
case DATEOID:
710+
{
711+
Datum ts = TimestampGetDatum(GetCurrentTimestamp());
712+
713+
default_value = perform_type_cast(ts, TIMESTAMPTZOID, DATEOID, NULL);
714+
}
715+
break;
716+
default:
717+
return false;
718+
}
719+
720+
/* Find suitable addition operator for default value and interval */
721+
op = get_binary_operator("+", atttype, interval_type);
722+
if (!op)
723+
elog(ERROR, "missing \"+\" operator for types %s and %s",
724+
format_type_be(atttype),
725+
format_type_be(interval_type));
726+
727+
op_func = oprfuncid(op);
728+
op_result_type = get_operator_ret_type(op);
729+
ReleaseSysCache(op);
730+
731+
/* Invoke addition operator and get a result*/
732+
op_result = OidFunctionCall2(op_func, default_value, interval);
733+
734+
/*
735+
* If operator result type isn't the same as original value then
736+
* convert it. We need this to make sure that specified interval would
737+
* change the _origianal_ value somehow. For example, if we add one second
738+
* to a date then we'll get a timestamp which is one second later than
739+
* original date (obviously). But when we convert it back to a date we will
740+
* get the same original value meaning that one second interval wouldn't
741+
* change original value anyhow. We should consider such interval
742+
* as trivial
743+
*/
744+
if (op_result_type != atttype)
745+
{
746+
op_result = perform_type_cast(op_result, op_result_type, atttype, NULL);
747+
op_result_type = atttype;
748+
}
749+
750+
/*
751+
* Compare it to the default_value. If they are the same then obviously
752+
* interval is trivial
753+
*/
754+
fill_type_cmp_fmgr_info(&cmp_func,
755+
getBaseType(atttype),
756+
getBaseType(op_result_type));
757+
if (DatumGetInt32(FunctionCall2(&cmp_func, default_value, op_result)) == 0)
758+
return true;
759+
760+
return false;
761+
}
762+
763+
652764
/*
653765
* ------------------
654766
* Helper functions

0 commit comments

Comments
 (0)