Skip to content

Commit dd9f391

Browse files
CherkashinSergeyza-arthur
authored andcommitted
Delete a variable if an error occurs during its creation
1 parent 5cdf6a0 commit dd9f391

File tree

5 files changed

+30
-5
lines changed

5 files changed

+30
-5
lines changed

expected/pg_variables_trans.out

+5
Original file line numberDiff line numberDiff line change
@@ -1854,6 +1854,11 @@ SELECT pgv_insert('package', 'errs',row(1), true);
18541854

18551855
(1 row)
18561856

1857+
-- Variable should not exists in case when error occurs during creation
1858+
SELECT pgv_insert('vars4', 'r1', row('str1', 'str1'));
1859+
ERROR: could not identify a hash function for type unknown
1860+
SELECT pgv_select('vars4', 'r1', 0);
1861+
ERROR: unrecognized variable "r1"
18571862
SELECT pgv_free();
18581863
pgv_free
18591864
----------

pg_variables.c

+8-5
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ static void copyValue(VarState *src, VarState *dest, Variable *destVar);
6868
static void freeValue(VarState *varstate, Oid typid);
6969
static void removeState(TransObject *object, TransObjectType type,
7070
TransState *stateToDelete);
71-
static void removeObject(TransObject *object, TransObjectType type);
7271
static bool isObjectChangedInCurrentTrans(TransObject *object);
7372
static bool isObjectChangedInUpperTrans(TransObject *object);
7473

@@ -1626,13 +1625,14 @@ copyValue(VarState *src, VarState *dest, Variable *destVar)
16261625
static void
16271626
freeValue(VarState *varstate, Oid typid)
16281627
{
1629-
if (typid == RECORDOID)
1628+
if (typid == RECORDOID && varstate->value.record.hctx)
16301629
{
16311630
/* All records will be freed */
16321631
MemoryContextDelete(varstate->value.record.hctx);
16331632
}
16341633
else if (varstate->value.scalar.typbyval == false &&
1635-
varstate->value.scalar.is_null == false)
1634+
varstate->value.scalar.is_null == false &&
1635+
varstate->value.scalar.value)
16361636
{
16371637
pfree(DatumGetPointer(varstate->value.scalar.value));
16381638
}
@@ -1651,7 +1651,8 @@ removeState(TransObject *object, TransObjectType type, TransState *stateToDelete
16511651
pfree(stateToDelete);
16521652
}
16531653

1654-
static void
1654+
/* Remove package or variable (either transactional or regular) */
1655+
void
16551656
removeObject(TransObject *object, TransObjectType type)
16561657
{
16571658
bool found;
@@ -1672,7 +1673,9 @@ removeObject(TransObject *object, TransObjectType type)
16721673
hash = packagesHash;
16731674
}
16741675
else
1675-
hash = ((Variable *) object)->package->varHashTransact;
1676+
hash = ((Variable *) object)->is_transactional ?
1677+
((Variable *) object)->package->varHashTransact :
1678+
((Variable *) object)->package->varHashRegular;
16761679

16771680
/* Remove all object's states */
16781681
while (!dlist_is_empty(&object->states))

pg_variables.h

+1
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ extern void check_record_key(Variable *variable, Oid typid);
155155
extern void insert_record(Variable *variable, HeapTupleHeader tupleHeader);
156156
extern bool update_record(Variable *variable, HeapTupleHeader tupleHeader);
157157
extern bool delete_record(Variable *variable, Datum value, bool is_null);
158+
extern void removeObject(TransObject *object, TransObjectType type);
158159

159160
#define GetActualState(object) \
160161
(dlist_head_element(TransState, node, &((TransObject *) object)->states))

pg_variables_record.c

+12
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,29 @@ init_record(RecordVar *record, TupleDesc tupdesc, Variable *variable)
8181
TYPECACHE_HASH_PROC_FINFO |
8282
TYPECACHE_CMP_PROC_FINFO);
8383

84+
/*
85+
* In case something went wrong, you need to roll back the changes before
86+
* completing the transaction, because the variable may be regular
87+
* and not present in list of changed vars.
88+
*/
8489
if (!OidIsValid(typentry->hash_proc_finfo.fn_oid))
90+
{
91+
/* At this point variable is just created, so we simply remove it. */
92+
removeObject(&variable->transObject, TRANS_VARIABLE);
8593
ereport(ERROR,
8694
(errcode(ERRCODE_UNDEFINED_FUNCTION),
8795
errmsg("could not identify a hash function for type %s",
8896
format_type_be(keyid))));
97+
}
8998

9099
if (!OidIsValid(typentry->cmp_proc_finfo.fn_oid))
100+
{
101+
removeObject(&variable->transObject, TRANS_VARIABLE);
91102
ereport(ERROR,
92103
(errcode(ERRCODE_UNDEFINED_FUNCTION),
93104
errmsg("could not identify a matching function for type %s",
94105
format_type_be(keyid))));
106+
}
95107

96108
/* Initialize the record */
97109

sql/pg_variables_trans.sql

+4
Original file line numberDiff line numberDiff line change
@@ -467,4 +467,8 @@ SELECT pgv_insert('package', 'errs',row(n), true)
467467
FROM generate_series(1,5) AS gs(n) WHERE 1.0/(n-3)<>0;
468468
SELECT pgv_insert('package', 'errs',row(1), true);
469469

470+
-- Variable should not exists in case when error occurs during creation
471+
SELECT pgv_insert('vars4', 'r1', row('str1', 'str1'));
472+
SELECT pgv_select('vars4', 'r1', 0);
473+
470474
SELECT pgv_free();

0 commit comments

Comments
 (0)