Skip to content

Commit 566b8f5

Browse files
funbringerfunbringer
committed
slightly reworked function add_to_pathman_config()
1 parent 083e6a1 commit 566b8f5

File tree

7 files changed

+60
-37
lines changed

7 files changed

+60
-37
lines changed

‎expected/pathman_calamity.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -553,9 +553,9 @@ SELECT add_to_pathman_config(NULL, 'val'); /* no table */
553553
ERROR: 'parent_relid' should not be NULL
554554
SELECT add_to_pathman_config(0::REGCLASS, 'val'); /* no table (oid) */
555555
ERROR: relation "0" does not exist
556-
SELECT add_to_pathman_config('calamity.part_test', NULL); /* no column */
556+
SELECT add_to_pathman_config('calamity.part_test', NULL); /* no expr */
557557
ERROR: 'expression' should not be NULL
558-
SELECT add_to_pathman_config('calamity.part_test', 'V_A_L'); /* wrong column */
558+
SELECT add_to_pathman_config('calamity.part_test', 'V_A_L'); /* wrong expr */
559559
ERROR: cannot find type name for attribute "v_a_l" of relation "part_test"
560560
SELECT add_to_pathman_config('calamity.part_test', 'val'); /* OK */
561561
add_to_pathman_config

‎hash.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ BEGIN
3636

3737
/* Insert new entry to pathman config */
3838
EXECUTE format('ANALYZE %s', parent_relid);
39-
PERFORM @extschema@.add_to_pathman_config(parent_relid, expression, NULL);
39+
PERFORM @extschema@.add_to_pathman_config(parent_relid, expression);
4040

4141
/* Create partitions */
4242
PERFORM @extschema@.create_hash_partitions_internal(parent_relid,

‎init.sql

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -857,15 +857,21 @@ LANGUAGE C STRICT;
857857

858858

859859
/*
860-
* Add record to pathman_config. If parttype if not specified then determine
861-
* partitioning type.
860+
* Add record to pathman_config (RANGE) and validate partitions.
862861
*/
863862
CREATE OR REPLACE FUNCTION @extschema@.add_to_pathman_config(
864863
parent_relid REGCLASS,
865864
expression TEXT,
866-
range_interval TEXT DEFAULT NULL,
867-
parttype INT4 DEFAULT 0
868-
)
865+
range_interval TEXT)
866+
RETURNS BOOLEAN AS 'pg_pathman', 'add_to_pathman_config'
867+
LANGUAGE C;
868+
869+
/*
870+
* Add record to pathman_config (HASH) and validate partitions.
871+
*/
872+
CREATE OR REPLACE FUNCTION @extschema@.add_to_pathman_config(
873+
parent_relid REGCLASS,
874+
expression TEXT)
869875
RETURNS BOOLEAN AS 'pg_pathman', 'add_to_pathman_config'
870876
LANGUAGE C;
871877

‎range.sql

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ BEGIN
156156
/* Insert new entry to pathman config */
157157
EXECUTE format('ANALYZE %s', parent_relid);
158158
PERFORM @extschema@.add_to_pathman_config(parent_relid, expression,
159-
p_interval::TEXT);
159+
p_interval::TEXT);
160160

161161
/* Create sequence for child partitions names */
162162
PERFORM @extschema@.create_or_replace_sequence(parent_relid);
@@ -254,7 +254,7 @@ BEGIN
254254
/* Insert new entry to pathman config */
255255
EXECUTE format('ANALYZE %s', parent_relid);
256256
PERFORM @extschema@.add_to_pathman_config(parent_relid, expression,
257-
p_interval::TEXT);
257+
p_interval::TEXT);
258258

259259
/* Create sequence for child partitions names */
260260
PERFORM @extschema@.create_or_replace_sequence(parent_relid);
@@ -313,7 +313,7 @@ BEGIN
313313

314314
/* Insert new entry to pathman config */
315315
EXECUTE format('ANALYZE %s', parent_relid);
316-
PERFORM @extschema@.add_to_pathman_config(parent_relid, expression, NULL, 2);
316+
PERFORM @extschema@.add_to_pathman_config(parent_relid, expression, NULL);
317317

318318
/* Create sequence for child partitions names */
319319
PERFORM @extschema@.create_or_replace_sequence(parent_relid);
@@ -365,7 +365,7 @@ BEGIN
365365
/* Insert new entry to pathman config */
366366
EXECUTE format('ANALYZE %s', parent_relid);
367367
PERFORM @extschema@.add_to_pathman_config(parent_relid, expression,
368-
p_interval::TEXT);
368+
p_interval::TEXT);
369369

370370
/* Create sequence for child partitions names */
371371
PERFORM @extschema@.create_or_replace_sequence(parent_relid);
@@ -423,7 +423,7 @@ BEGIN
423423
/* Insert new entry to pathman config */
424424
EXECUTE format('ANALYZE %s', parent_relid);
425425
PERFORM @extschema@.add_to_pathman_config(parent_relid, expression,
426-
p_interval::TEXT);
426+
p_interval::TEXT);
427427

428428
/* Create sequence for child partitions names */
429429
PERFORM @extschema@.create_or_replace_sequence(parent_relid);

‎sql/pathman_calamity.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,8 @@ DROP FUNCTION calamity.dummy_cb(arg jsonb);
236236
/* check function add_to_pathman_config() -- PHASE #1 */
237237
SELECT add_to_pathman_config(NULL, 'val'); /* no table */
238238
SELECT add_to_pathman_config(0::REGCLASS, 'val'); /* no table (oid) */
239-
SELECT add_to_pathman_config('calamity.part_test', NULL); /* no column */
240-
SELECT add_to_pathman_config('calamity.part_test', 'V_A_L'); /* wrong column */
239+
SELECT add_to_pathman_config('calamity.part_test', NULL); /* no expr */
240+
SELECT add_to_pathman_config('calamity.part_test', 'V_A_L'); /* wrong expr */
241241
SELECT add_to_pathman_config('calamity.part_test', 'val'); /* OK */
242242
SELECT disable_pathman_for('calamity.part_test');
243243
SELECT add_to_pathman_config('calamity.part_test', 'val', '10'); /* OK */

‎src/init.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,7 @@ pathman_config_contains_relation(Oid relid, Datum *values, bool *isnull,
619619
/* Open PATHMAN_CONFIG with latest snapshot available */
620620
rel = heap_open(get_pathman_config_relid(false), AccessShareLock);
621621

622-
/* Check that 'partrel' column is if regclass type */
622+
/* Check that 'partrel' column is of regclass type */
623623
Assert(RelationGetDescr(rel)->
624624
attrs[Anum_pathman_config_partrel - 1]->
625625
atttypid == REGCLASSOID);

‎src/pl_funcs.c

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -735,29 +735,57 @@ add_to_pathman_config(PG_FUNCTION_ARGS)
735735
else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
736736
errmsg("'expression' should not be NULL")));
737737

738+
/* Check current user's privileges */
738739
if (!check_security_policy_internal(relid, GetUserId()))
739740
{
740-
elog(ERROR, "only the owner or superuser can change "
741-
"partitioning configuration of table \"%s\"",
742-
get_rel_name_or_relid(relid));
741+
ereport(ERROR,
742+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
743+
errmsg("only the owner or superuser can change "
744+
"partitioning configuration of table \"%s\"",
745+
get_rel_name_or_relid(relid))));
743746
}
744747

745748
/* Select partitioning type */
746-
parttype = PG_GETARG_INT32(3);
747-
if ((parttype != PT_HASH) && (parttype != PT_RANGE))
748-
parttype = PG_ARGISNULL(2) ? PT_HASH : PT_RANGE;
749+
switch (PG_NARGS())
750+
{
751+
/* HASH */
752+
case 2:
753+
{
754+
parttype = PT_HASH;
755+
756+
values[Anum_pathman_config_range_interval - 1] = (Datum) 0;
757+
isnull[Anum_pathman_config_range_interval - 1] = true;
758+
}
759+
break;
760+
761+
/* RANGE */
762+
case 3:
763+
{
764+
parttype = PT_RANGE;
765+
766+
values[Anum_pathman_config_range_interval - 1] = PG_GETARG_DATUM(2);
767+
isnull[Anum_pathman_config_range_interval - 1] = PG_ARGISNULL(2);
768+
}
769+
break;
770+
771+
default:
772+
elog(ERROR, "error in function " CppAsString(add_to_pathman_config));
773+
PG_RETURN_BOOL(false); /* keep compiler happy */
774+
}
749775

750776
/* Parse and check expression */
751777
expr_datum = plan_partitioning_expression(relid, expression, &expr_type);
752778

753-
/* Expression for range partitions should be hashable */
779+
/* Check hash function for HASH partitioning */
754780
if (parttype == PT_HASH)
755781
{
756782
TypeCacheEntry *tce;
757783

758784
tce = lookup_type_cache(expr_type, TYPECACHE_HASH_PROC);
759-
if (tce->hash_proc == InvalidOid)
760-
elog(ERROR, "partitioning expression should be hashable");
785+
if (!OidIsValid(tce->hash_proc))
786+
ereport(ERROR,
787+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
788+
errmsg("no hash function for partitioning expression")));
761789
}
762790

763791
/*
@@ -778,17 +806,6 @@ add_to_pathman_config(PG_FUNCTION_ARGS)
778806
values[Anum_pathman_config_atttype - 1] = ObjectIdGetDatum(expr_type);
779807
isnull[Anum_pathman_config_atttype - 1] = false;
780808

781-
if (parttype == PT_RANGE)
782-
{
783-
values[Anum_pathman_config_range_interval - 1] = PG_GETARG_DATUM(2);
784-
isnull[Anum_pathman_config_range_interval - 1] = PG_ARGISNULL(2);
785-
}
786-
else
787-
{
788-
values[Anum_pathman_config_range_interval - 1] = (Datum) 0;
789-
isnull[Anum_pathman_config_range_interval - 1] = true;
790-
}
791-
792809
/* Insert new row into PATHMAN_CONFIG */
793810
pathman_config = heap_open(get_pathman_config_relid(false), RowExclusiveLock);
794811

@@ -798,7 +815,7 @@ add_to_pathman_config(PG_FUNCTION_ARGS)
798815

799816
heap_close(pathman_config, RowExclusiveLock);
800817

801-
/* update caches only if this relation has children */
818+
/* Update caches only if this relation has children */
802819
if (has_subclass(relid))
803820
{
804821
/* Now try to create a PartRelationInfo */

0 commit comments

Comments
 (0)