Skip to content

Commit 0c7a850

Browse files
CherkashinSergeyCherkashinSergey
committed
Fix pgv_stats() segfaults
1 parent 5b8b4bc commit 0c7a850

File tree

3 files changed

+65
-52
lines changed

3 files changed

+65
-52
lines changed

‎expected/pg_variables_trans.out

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1752,53 +1752,52 @@ SELECT pgv_set('vars2', 'trans', 'variable exists'::text, true);
17521752

17531753
SAVEPOINT sp4;
17541754
SAVEPOINT sp5;
1755-
--SELECT pgv_remove('vars2');
17561755
SELECT pgv_free();
17571756
pgv_free
17581757
----------
17591758

17601759
(1 row)
17611760

1762-
SELECT pgv_stats();
1763-
pgv_stats
1764-
---------------
1765-
(vars2,24576)
1766-
(vars,24576)
1761+
SELECT package FROM pgv_stats();
1762+
package
1763+
---------
1764+
vars2
1765+
vars
17671766
(2 rows)
17681767

1769-
SELECT pgv_list();
1770-
pgv_list
1771-
----------
1768+
SELECT * FROM pgv_list() ORDER BY package, name;
1769+
package | name | is_transactional
1770+
---------+------+------------------
17721771
(0 rows)
17731772

17741773
RELEASE sp5;
1775-
SELECT pgv_stats();
1776-
pgv_stats
1777-
---------------
1778-
(vars2,24576)
1779-
(vars,24576)
1774+
SELECT package FROM pgv_stats();
1775+
package
1776+
---------
1777+
vars2
1778+
vars
17801779
(2 rows)
17811780

1782-
SELECT pgv_list();
1783-
pgv_list
1784-
----------
1781+
SELECT * FROM pgv_list() ORDER BY package, name;
1782+
package | name | is_transactional
1783+
---------+------+------------------
17851784
(0 rows)
17861785

17871786
RELEASE sp4;
1788-
SELECT pgv_stats();
1789-
pgv_stats
1790-
--------------
1791-
(vars,24576)
1787+
SELECT package FROM pgv_stats();
1788+
package
1789+
---------
1790+
vars
17921791
(1 row)
17931792

1794-
SELECT pgv_list();
1795-
pgv_list
1796-
----------
1793+
SELECT * FROM pgv_list() ORDER BY package, name;
1794+
package | name | is_transactional
1795+
---------+------+------------------
17971796
(0 rows)
17981797

17991798
COMMIT;
1800-
SELECT pgv_stats();
1801-
pgv_stats
1802-
-----------
1799+
SELECT package FROM pgv_stats();
1800+
package
1801+
---------
18031802
(0 rows)
18041803

‎pg_variables.c

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,11 @@ static HashVariableEntry *LastVariable = NULL;
107107
static dlist_head *changesStack = NULL;
108108
static MemoryContext changesStackContext = NULL;
109109

110-
/* Returns a list of of vars changed at current subxact level */
111-
#define get_actual_changed_vars_list() \
110+
/* Returns a lists of packages and variables changed at current subxact level */
111+
#define get_actual_changes_list() \
112112
( \
113113
AssertMacro(changesStack != NULL), \
114-
(dlist_head_element(ChangesStackNode, \
115-
node, changesStack))->changedVarsList \
114+
(dlist_head_element(ChangesStackNode, node, changesStack)) \
116115
)
117116

118117

@@ -1192,8 +1191,8 @@ get_packages_stats(PG_FUNCTION_ARGS)
11921191

11931192
/* Fill data */
11941193
values[0] = PointerGetDatum(cstring_to_text(package->name));
1195-
1196-
getMemoryTotalSpace(package->hctxRegular, 0, &regularSpace);
1194+
if (get_actual_pack_state(package)->is_valid)
1195+
getMemoryTotalSpace(package->hctxRegular, 0, &regularSpace);
11971196
getMemoryTotalSpace(package->hctxTransact, 0, &transactSpace);
11981197
totalSpace = regularSpace + transactSpace;
11991198
values[1] = Int64GetDatum(totalSpace);
@@ -1535,7 +1534,7 @@ createSavepointVar(HashPackageEntry *package, HashVariableEntry *variable)
15351534

15361535
/* Release memory for variable */
15371536
history_entry_new = palloc0(sizeof(ValueHistoryEntry));
1538-
history_entry_prev = dlist_head_element(ValueHistoryEntry, node, history);
1537+
history_entry_prev = get_actual_var_state(variable);
15391538
scalar = &history_entry_new->value.scalar;
15401539
*scalar = history_entry_prev->value.scalar;
15411540

@@ -1626,7 +1625,7 @@ createSavepointPack(HashPackageEntry *package)
16261625

16271626
history = &package->packHistory;
16281627
history_entry_new = MemoryContextAllocZero(ModuleContext, sizeof(PackHistoryEntry));
1629-
history_entry_prev = dlist_head_element(PackHistoryEntry, node, history);
1628+
history_entry_prev = get_actual_pack_state(package);
16301629
history_entry_new->is_valid = history_entry_prev->is_valid;
16311630
dlist_push_head(history, &history_entry_new->node);
16321631
}
@@ -1666,8 +1665,8 @@ releaseSavepointPack(HashPackageEntry *package)
16661665
/* Remove package from packagesHash */
16671666
hash_search(packagesHash, package->name, HASH_REMOVE, &found);
16681667
/*
1669-
*Delete a variable from the change history of the overlying
1670-
*transaction level.
1668+
* Delete a variable from the change history of the overlying
1669+
* transaction level (head of 'changesStack' at this point)
16711670
*/
16721671
if (!dlist_is_empty(changesStack))
16731672
removeFromChangedVars(package);
@@ -1865,7 +1864,7 @@ addToChangedPacks(HashPackageEntry *package)
18651864
{
18661865
ChangedPacksNode *cpn;
18671866

1868-
csn = dlist_head_element(ChangesStackNode, node, changesStack);
1867+
csn = get_actual_changes_list();
18691868
cpn = makeChangedPacksNode(csn->ctx, package);
18701869
dlist_push_head(csn->changedPacksList, &cpn->node);
18711870

@@ -1898,7 +1897,7 @@ addToChangedVars(HashPackageEntry *package, HashVariableEntry *variable)
18981897
{
18991898
ChangedVarsNode *cvn;
19001899

1901-
csn = dlist_head_element(ChangesStackNode, node, changesStack);
1900+
csn = get_actual_changes_list();
19021901
cvn = makeChangedVarsNode(csn->ctx, package, variable);
19031902
dlist_push_head(csn->changedVarsList, &cvn->node);
19041903

@@ -1908,21 +1907,37 @@ addToChangedVars(HashPackageEntry *package, HashVariableEntry *variable)
19081907
}
19091908

19101909
/*
1911-
* Remove from the changes list the variables of the deleted package
1910+
* Remove from the changes list a deleted package
19121911
*/
19131912
static void
19141913
removeFromChangedVars(HashPackageEntry *package)
19151914
{
1916-
dlist_mutable_iter var_miter;
1917-
dlist_head *changedVarsList;
1915+
dlist_mutable_iter var_miter,
1916+
pack_miter;
1917+
dlist_head *changedVarsList,
1918+
*changedPacksList;
19181919

1919-
changedVarsList = get_actual_changed_vars_list();
1920+
/* First remove corresponding variables from changedVarsList */
1921+
changedVarsList = get_actual_changes_list()->changedVarsList;
19201922
dlist_foreach_modify(var_miter, changedVarsList)
19211923
{
1922-
ChangedVarsNode *cvn_cur = dlist_container(ChangedVarsNode, node, var_miter.cur);
1924+
ChangedVarsNode *cvn_cur = dlist_container(ChangedVarsNode, node,
1925+
var_miter.cur);
19231926
if (cvn_cur->package == package)
19241927
dlist_delete(&cvn_cur->node);
19251928
}
1929+
/* Now remove package itself from changedPacksList */
1930+
changedPacksList = get_actual_changes_list()->changedPacksList;
1931+
dlist_foreach_modify(pack_miter, changedPacksList)
1932+
{
1933+
ChangedPacksNode *cpn_cur = dlist_container(ChangedPacksNode, node,
1934+
pack_miter.cur);
1935+
if (cpn_cur->package == package)
1936+
{
1937+
dlist_delete(&cpn_cur->node);
1938+
break;
1939+
}
1940+
}
19261941
}
19271942

19281943
/*

‎sql/pg_variables_trans.sql

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -440,15 +440,14 @@ SAVEPOINT sp3;
440440
SELECT pgv_set('vars2', 'trans', 'variable exists'::text, true);
441441
SAVEPOINT sp4;
442442
SAVEPOINT sp5;
443-
--SELECT pgv_remove('vars2');
444443
SELECT pgv_free();
445-
SELECT pgv_stats();
446-
SELECT pgv_list();
444+
SELECT package FROM pgv_stats();
445+
SELECT * FROM pgv_list() ORDER BY package, name;
447446
RELEASE sp5;
448-
SELECT pgv_stats();
449-
SELECT pgv_list();
447+
SELECT package FROM pgv_stats();
448+
SELECT * FROM pgv_list() ORDER BY package, name;
450449
RELEASE sp4;
451-
SELECT pgv_stats();
452-
SELECT pgv_list();
450+
SELECT package FROM pgv_stats();
451+
SELECT * FROM pgv_list() ORDER BY package, name;
453452
COMMIT;
454-
SELECT pgv_stats();
453+
SELECT package FROM pgv_stats();

0 commit comments

Comments
 (0)