Skip to content

Commit c0a1bb8

Browse files
funbringerfunbringer
committed
clean code in pl_hash_funcs.c & pl_range_funcs.c
1 parent 609813b commit c0a1bb8

File tree

2 files changed

+123
-91
lines changed

2 files changed

+123
-91
lines changed

‎src/pl_hash_funcs.c

Lines changed: 96 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
#include "utils/array.h"
2323

2424

25+
static char **deconstruct_text_array(Datum arr, int *num_elems);
26+
27+
2528
/* Function declarations */
2629

2730
PG_FUNCTION_INFO_V1( create_hash_partitions_internal );
@@ -32,112 +35,85 @@ PG_FUNCTION_INFO_V1( get_hash_part_idx );
3235
PG_FUNCTION_INFO_V1( build_hash_condition );
3336

3437

35-
static char **deconstruct_text_array(Datum arr, int *num_elems);
36-
37-
3838
/*
3939
* Create HASH partitions implementation (written in C).
4040
*/
4141
Datum
4242
create_hash_partitions_internal(PG_FUNCTION_ARGS)
4343
{
44-
Oid parent_relid = PG_GETARG_OID(0);
45-
Datum partitioned_col_name = PG_GETARG_DATUM(1);
46-
Oid partitioned_col_type;
47-
uint32 part_count = PG_GETARG_INT32(2),
48-
i;
49-
50-
/* Partitions names and tablespaces */
51-
char **names = NULL,
52-
**tablespaces = NULL;
53-
int names_size = 0,
54-
tablespaces_size = 0;
55-
RangeVar **rangevars = NULL;
44+
/* Free allocated arrays */
45+
#define DeepFreeArray(arr, arr_len) \
46+
do { \
47+
int arr_elem; \
48+
if (!arr) break; \
49+
for (arr_elem = 0; arr_elem < arr_len; arr_elem++) \
50+
pfree(arr[arr_elem]); \
51+
pfree(arr); \
52+
} while (0)
53+
54+
Oid parent_relid = PG_GETARG_OID(0);
55+
const char *partitioned_col_name = TextDatumGetCString(PG_GETARG_DATUM(1));
56+
Oid partitioned_col_type;
57+
uint32 part_count = PG_GETARG_INT32(2),
58+
i;
59+
60+
/* Partition names and tablespaces */
61+
char **relnames = NULL,
62+
**tablespaces = NULL;
63+
int relnames_size = 0,
64+
tablespaces_size = 0;
65+
RangeVar **rangevars = NULL;
5666

5767
/* Check that there's no partitions yet */
5868
if (get_pathman_relation_info(parent_relid))
5969
elog(ERROR, "cannot add new HASH partitions");
6070

6171
partitioned_col_type = get_attribute_type(parent_relid,
62-
TextDatumGetCString(partitioned_col_name),
72+
partitioned_col_name,
6373
false);
6474

65-
/* Get partition names and tablespaces */
75+
/* Extract partition names */
6676
if (!PG_ARGISNULL(3))
67-
names = deconstruct_text_array(PG_GETARG_DATUM(3), &names_size);
77+
relnames = deconstruct_text_array(PG_GETARG_DATUM(3), &relnames_size);
6878

79+
/* Extract partition tablespaces */
6980
if (!PG_ARGISNULL(4))
7081
tablespaces = deconstruct_text_array(PG_GETARG_DATUM(4), &tablespaces_size);
7182

83+
/* If both arrays are present, check that their lengths are equal */
84+
if (relnames && tablespaces && relnames_size != tablespaces_size)
85+
elog(ERROR, "sizes of arrays 'relnames' and 'tablespaces' are different");
86+
7287
/* Convert partition names into RangeVars */
73-
if (names_size > 0)
88+
if (relnames)
7489
{
75-
rangevars = palloc(sizeof(RangeVar) * names_size);
76-
for (i = 0; i < names_size; i++)
90+
rangevars = palloc(sizeof(RangeVar) * relnames_size);
91+
for (i = 0; i < relnames_size; i++)
7792
{
78-
List *nl = stringToQualifiedNameList(names[i]);
93+
List *nl = stringToQualifiedNameList(relnames[i]);
7994

8095
rangevars[i] = makeRangeVarFromNameList(nl);
8196
}
8297
}
8398

99+
/* Finally create HASH partitions */
84100
for (i = 0; i < part_count; i++)
85101
{
86-
RangeVar *rel = rangevars != NULL ? rangevars[i] : NULL;
87-
char *tablespace = tablespaces != NULL ? tablespaces[i] : NULL;
102+
RangeVar *partition_rv = rangevars ? rangevars[i] : NULL;
103+
char *tablespace = tablespaces ? tablespaces[i] : NULL;
88104

89105
/* Create a partition (copy FKs, invoke callbacks etc) */
90106
create_single_hash_partition_internal(parent_relid, i, part_count,
91107
partitioned_col_type,
92-
rel, tablespace);
108+
partition_rv, tablespace);
93109
}
94110

95-
PG_RETURN_VOID();
96-
}
97-
98-
/*
99-
* Convert Datum into cstring array
100-
*/
101-
static char **
102-
deconstruct_text_array(Datum arr, int *num_elems)
103-
{
104-
ArrayType *arrayval;
105-
int16 elemlen;
106-
bool elembyval;
107-
char elemalign;
108-
Datum *elem_values;
109-
bool *elem_nulls;
110-
int16 i;
111-
112-
arrayval = DatumGetArrayTypeP(arr);
113-
114-
Assert(ARR_ELEMTYPE(arrayval) == TEXTOID);
115-
116-
get_typlenbyvalalign(ARR_ELEMTYPE(arrayval),
117-
&elemlen, &elembyval, &elemalign);
118-
deconstruct_array(arrayval,
119-
ARR_ELEMTYPE(arrayval),
120-
elemlen, elembyval, elemalign,
121-
&elem_values, &elem_nulls, num_elems);
122-
123-
/* If there are actual values then convert them into cstrings */
124-
if (num_elems > 0)
125-
{
126-
char **strings = palloc(sizeof(char *) * *num_elems);
127-
128-
for (i = 0; i < *num_elems; i++)
129-
{
130-
if (elem_nulls[i])
131-
elog(ERROR,
132-
"Partition name and tablespace arrays cannot contain nulls");
133-
134-
strings[i] = TextDatumGetCString(elem_values[i]);
135-
}
136-
137-
return strings;
138-
}
111+
/* Free arrays */
112+
DeepFreeArray(relnames, relnames_size);
113+
DeepFreeArray(tablespaces, tablespaces_size);
114+
DeepFreeArray(rangevars, relnames_size);
139115

140-
return NULL;
116+
PG_RETURN_VOID();
141117
}
142118

143119
/*
@@ -201,3 +177,53 @@ build_hash_condition(PG_FUNCTION_ARGS)
201177

202178
PG_RETURN_TEXT_P(cstring_to_text(result));
203179
}
180+
181+
182+
/*
183+
* ------------------
184+
* Helper functions
185+
* ------------------
186+
*/
187+
188+
/* Convert Datum into CSTRING array */
189+
static char **
190+
deconstruct_text_array(Datum arr, int *num_elems)
191+
{
192+
ArrayType *arrayval;
193+
int16 elemlen;
194+
bool elembyval;
195+
char elemalign;
196+
Datum *elem_values;
197+
bool *elem_nulls;
198+
int16 i;
199+
200+
arrayval = DatumGetArrayTypeP(arr);
201+
202+
Assert(ARR_ELEMTYPE(arrayval) == TEXTOID);
203+
204+
get_typlenbyvalalign(ARR_ELEMTYPE(arrayval),
205+
&elemlen, &elembyval, &elemalign);
206+
deconstruct_array(arrayval,
207+
ARR_ELEMTYPE(arrayval),
208+
elemlen, elembyval, elemalign,
209+
&elem_values, &elem_nulls, num_elems);
210+
211+
/* If there are actual values then convert them into CSTRINGs */
212+
if (num_elems > 0)
213+
{
214+
char **strings = palloc(sizeof(char *) * *num_elems);
215+
216+
for (i = 0; i < *num_elems; i++)
217+
{
218+
if (elem_nulls[i])
219+
elog(ERROR, "partition name and tablespace arrays "
220+
"may not contain nulls");
221+
222+
strings[i] = TextDatumGetCString(elem_values[i]);
223+
}
224+
225+
return strings;
226+
}
227+
228+
return NULL;
229+
}

‎src/pl_range_funcs.c

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ static void drop_table_by_oid(Oid relid);
5353
/* Function declarations */
5454

5555
PG_FUNCTION_INFO_V1( create_single_range_partition_pl );
56-
PG_FUNCTION_INFO_V1( find_or_create_range_partition);
56+
PG_FUNCTION_INFO_V1( find_or_create_range_partition );
5757
PG_FUNCTION_INFO_V1( check_range_available_pl );
5858

5959
PG_FUNCTION_INFO_V1( get_part_range_by_oid );
@@ -322,6 +322,7 @@ get_part_range_by_idx(PG_FUNCTION_ARGS)
322322

323323
ranges = PrelGetRangesArray(prel);
324324

325+
/* Build args for construct_infinitable_array() */
325326
elems[0] = ranges[partition_idx].min;
326327
elems[1] = ranges[partition_idx].max;
327328

@@ -369,6 +370,7 @@ build_range_condition(PG_FUNCTION_ARGS)
369370
PG_RETURN_TEXT_P(cstring_to_text(result));
370371
}
371372

373+
/* Build name for sequence for auto partition naming */
372374
Datum
373375
build_sequence_name(PG_FUNCTION_ARGS)
374376
{
@@ -544,8 +546,8 @@ merge_range_partitions_internal(Oid parent, Oid *parts, uint32 nparts)
544546

545547

546548
/*
547-
* Drops partition and expands the next partition so that it cover dropped
548-
* one
549+
* Drops partition and expands the next partition
550+
* so that it could cover the dropped one
549551
*
550552
* This function was written in order to support Oracle-like ALTER TABLE ...
551553
* DROP PARTITION. In Oracle partitions only have upper bound and when
@@ -554,49 +556,54 @@ merge_range_partitions_internal(Oid parent, Oid *parts, uint32 nparts)
554556
Datum
555557
drop_range_partition_expand_next(PG_FUNCTION_ARGS)
556558
{
557-
PartParentSearch parent_search;
558559
const PartRelationInfo *prel;
559-
RangeEntry *ranges;
560-
Oid relid = PG_GETARG_OID(0),
561-
parent;
562-
int i;
560+
PartParentSearch parent_search;
561+
Oid relid = PG_GETARG_OID(0),
562+
parent;
563+
RangeEntry *ranges;
564+
int i;
563565

564-
/* Get parent relid */
566+
/* Get parent's relid */
565567
parent = get_parent_of_partition(relid, &parent_search);
566568
if (parent_search != PPS_ENTRY_PART_PARENT)
567569
elog(ERROR, "relation \"%s\" is not a partition",
568570
get_rel_name_or_relid(relid));
569571

572+
/* Fetch PartRelationInfo and perform some checks */
570573
prel = get_pathman_relation_info(parent);
571574
shout_if_prel_is_invalid(parent, prel, PT_RANGE);
572575

576+
/* Fetch ranges array */
573577
ranges = PrelGetRangesArray(prel);
574578

575579
/* Looking for partition in child relations */
576-
for (i = 0; i < prel->children_count; i++)
580+
for (i = 0; i < PrelChildrenCount(prel); i++)
577581
if (ranges[i].child_oid == relid)
578582
break;
579583

580584
/*
581-
* It must be in ranges array because we already know that table
582-
* is a partition
585+
* It must be in ranges array because we already
586+
* know that this table is a partition
583587
*/
584-
Assert(i < prel->children_count);
588+
Assert(i < PrelChildrenCount(prel));
585589

586-
/* If there is next partition then expand it */
587-
if (i < prel->children_count - 1)
590+
/* Expand next partition if it exists */
591+
if (i < PrelChildrenCount(prel) - 1)
588592
{
589593
RangeEntry *cur = &ranges[i],
590594
*next = &ranges[i + 1];
591595

596+
/* Drop old constraint and create a new one */
592597
modify_range_constraint(next->child_oid,
593-
get_relid_attribute_name(prel->key, prel->attnum),
594-
prel->attnum,
595-
prel->atttype,
596-
&cur->min,
597-
&next->max);
598+
get_relid_attribute_name(prel->key,
599+
prel->attnum),
600+
prel->attnum,
601+
prel->atttype,
602+
&cur->min,
603+
&next->max);
598604
}
599605

606+
/* Finally drop this partition */
600607
drop_table_by_oid(relid);
601608

602609
PG_RETURN_VOID();
@@ -646,7 +653,6 @@ validate_interval_value(PG_FUNCTION_ARGS)
646653
* ------------------
647654
* Helper functions
648655
* ------------------
649-
*
650656
*/
651657

652658
/*

0 commit comments

Comments
 (0)