Skip to content

Commit 75ffb44

Browse files
jconwayjconway
committed
Have crosstab variants treat NULL rowid as a category in its own right,
per suggestion from Tom Lane. This fixes crash-bug reported by Stefan Schwarzer.
1 parent b7f1fe6 commit 75ffb44

File tree

3 files changed

+91
-58
lines changed

3 files changed

+91
-58
lines changed

‎contrib/tablefunc/data/ct.data

+4
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,7 @@
1212
12 group2 test4 att1 val4
1313
13 group2 test4 att2 val5
1414
14 group2 test4 att3 val6
15+
15 group1 \N att1 val9
16+
16 group1 \N att2 val10
17+
17 group1 \N att3 val11
18+
18 group1 \N att4 val12

‎contrib/tablefunc/expected/tablefunc.out

+28-19
Original file line numberDiff line numberDiff line change
@@ -23,42 +23,48 @@ SELECT * FROM crosstab2('SELECT rowid, attribute, val FROM ct where rowclass = '
2323
----------+------------+------------
2424
test1 | val2 | val3
2525
test2 | val6 | val7
26-
(2 rows)
26+
| val10 | val11
27+
(3 rows)
2728

2829
SELECT * FROM crosstab3('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or attribute = ''att3'') ORDER BY 1,2;');
2930
row_name | category_1 | category_2 | category_3
3031
----------+------------+------------+------------
3132
test1 | val2 | val3 |
3233
test2 | val6 | val7 |
33-
(2 rows)
34+
| val10 | val11 |
35+
(3 rows)
3436

3537
SELECT * FROM crosstab4('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or attribute = ''att3'') ORDER BY 1,2;');
3638
row_name | category_1 | category_2 | category_3 | category_4
3739
----------+------------+------------+------------+------------
3840
test1 | val2 | val3 | |
3941
test2 | val6 | val7 | |
40-
(2 rows)
42+
| val10 | val11 | |
43+
(3 rows)
4144

4245
SELECT * FROM crosstab2('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
4346
row_name | category_1 | category_2
4447
----------+------------+------------
4548
test1 | val1 | val2
4649
test2 | val5 | val6
47-
(2 rows)
50+
| val9 | val10
51+
(3 rows)
4852

4953
SELECT * FROM crosstab3('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
5054
row_name | category_1 | category_2 | category_3
5155
----------+------------+------------+------------
5256
test1 | val1 | val2 | val3
5357
test2 | val5 | val6 | val7
54-
(2 rows)
58+
| val9 | val10 | val11
59+
(3 rows)
5560

5661
SELECT * FROM crosstab4('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
5762
row_name | category_1 | category_2 | category_3 | category_4
5863
----------+------------+------------+------------+------------
5964
test1 | val1 | val2 | val3 | val4
6065
test2 | val5 | val6 | val7 | val8
61-
(2 rows)
66+
| val9 | val10 | val11 | val12
67+
(3 rows)
6268

6369
SELECT * FROM crosstab2('SELECT rowid, attribute, val FROM ct where rowclass = ''group2'' and (attribute = ''att1'' or attribute = ''att2'') ORDER BY 1,2;');
6470
row_name | category_1 | category_2
@@ -103,25 +109,28 @@ SELECT * FROM crosstab4('SELECT rowid, attribute, val FROM ct where rowclass = '
103109
(2 rows)
104110

105111
SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;', 2) AS c(rowid text, att1 text, att2 text);
106-
rowid | att1 | att2
107-
-------+------+------
112+
rowid | att1 | att2
113+
-------+------+-------
108114
test1 | val1 | val2
109115
test2 | val5 | val6
110-
(2 rows)
116+
| val9 | val10
117+
(3 rows)
111118

112119
SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;', 3) AS c(rowid text, att1 text, att2 text, att3 text);
113-
rowid | att1 | att2 | att3
114-
-------+------+------+------
115-
test1 | val1 | val2 | val3
116-
test2 | val5 | val6 | val7
117-
(2 rows)
120+
rowid | att1 | att2 | att3
121+
-------+------+-------+-------
122+
test1 | val1 | val2 | val3
123+
test2 | val5 | val6 | val7
124+
| val9 | val10 | val11
125+
(3 rows)
118126

119127
SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;', 4) AS c(rowid text, att1 text, att2 text, att3 text, att4 text);
120-
rowid | att1 | att2 | att3 | att4
121-
-------+------+------+------+------
122-
test1 | val1 | val2 | val3 | val4
123-
test2 | val5 | val6 | val7 | val8
124-
(2 rows)
128+
rowid | att1 | att2 | att3 | att4
129+
-------+------+-------+-------+-------
130+
test1 | val1 | val2 | val3 | val4
131+
test2 | val5 | val6 | val7 | val8
132+
| val9 | val10 | val11 | val12
133+
(3 rows)
125134

126135
-- test connectby with text based hierarchy
127136
CREATE TABLE connectby_text(keyid text, parent_keyid text);

‎contrib/tablefunc/tablefunc.c

+59-39
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,18 @@ typedef struct
9393
} \
9494
} while (0)
9595

96+
#define xpstrdup(tgtvar_, srcvar_) \
97+
do { \
98+
if (srcvar_) \
99+
tgtvar_ = pstrdup(srcvar_); \
100+
else \
101+
tgtvar_ = NULL; \
102+
} while (0)
103+
104+
#define xstreq(tgtvar_, srcvar_) \
105+
(((tgtvar_ == NULL) && (srcvar_ == NULL)) || \
106+
((tgtvar_ != NULL) && (srcvar_ != NULL) && (strcmp(tgtvar_, srcvar_) == 0)))
107+
96108
/* sign, 10 digits, '\0' */
97109
#define INT32_STRLEN 12
98110

@@ -299,6 +311,7 @@ crosstab(PG_FUNCTION_ARGS)
299311
crosstab_fctx *fctx;
300312
int i;
301313
int num_categories;
314+
bool firstpass = false;
302315
MemoryContext oldcontext;
303316

304317
/* stuff done only on the first call of the function */
@@ -420,6 +433,7 @@ crosstab(PG_FUNCTION_ARGS)
420433
funcctx->max_calls = proc;
421434

422435
MemoryContextSwitchTo(oldcontext);
436+
firstpass = true;
423437
}
424438

425439
/* stuff done on every call of the function */
@@ -454,7 +468,7 @@ crosstab(PG_FUNCTION_ARGS)
454468
HeapTuple tuple;
455469
Datum result;
456470
char **values;
457-
bool allnulls = true;
471+
bool skip_tuple = false;
458472

459473
while (true)
460474
{
@@ -485,69 +499,72 @@ crosstab(PG_FUNCTION_ARGS)
485499

486500
/*
487501
* If this is the first pass through the values for this
488-
* rowid set it, otherwise make sure it hasn't changed on
489-
* us. Also check to see if the rowid is the same as that
490-
* of the last tuple sent -- if so, skip this tuple
491-
* entirely
502+
* rowid, set the first column to rowid
492503
*/
493504
if (i == 0)
494-
values[0] = pstrdup(rowid);
495-
496-
if ((rowid != NULL) && (strcmp(rowid, values[0]) == 0))
497505
{
498-
if ((lastrowid != NULL) && (strcmp(rowid, lastrowid) == 0))
506+
xpstrdup(values[0], rowid);
507+
508+
/*
509+
* Check to see if the rowid is the same as that of the last
510+
* tuple sent -- if so, skip this tuple entirely
511+
*/
512+
if (!firstpass && xstreq(lastrowid, rowid))
513+
{
514+
skip_tuple = true;
499515
break;
500-
else if (allnulls == true)
501-
allnulls = false;
516+
}
517+
}
502518

519+
/*
520+
* If rowid hasn't changed on us, continue building the
521+
* ouput tuple.
522+
*/
523+
if (xstreq(rowid, values[0]))
524+
{
503525
/*
504-
* Get the next category item value, which is alway
526+
* Get the next category item value, which is always
505527
* attribute number three.
506528
*
507-
* Be careful to sssign the value to the array index
508-
* based on which category we are presently
509-
* processing.
529+
* Be careful to assign the value to the array index based
530+
* on which category we are presently processing.
510531
*/
511532
values[1 + i] = SPI_getvalue(spi_tuple, spi_tupdesc, 3);
512533

513534
/*
514-
* increment the counter since we consume a row for
515-
* each category, but not for last pass because the
516-
* API will do that for us
535+
* increment the counter since we consume a row for each
536+
* category, but not for last pass because the API will do
537+
* that for us
517538
*/
518539
if (i < (num_categories - 1))
519540
call_cntr = ++funcctx->call_cntr;
520541
}
521542
else
522543
{
523544
/*
524-
* We'll fill in NULLs for the missing values, but we
525-
* need to decrement the counter since this sql result
526-
* row doesn't belong to the current output tuple.
545+
* We'll fill in NULLs for the missing values, but we need
546+
* to decrement the counter since this sql result row
547+
* doesn't belong to the current output tuple.
527548
*/
528549
call_cntr = --funcctx->call_cntr;
529550
break;
530551
}
531-
532-
if (rowid != NULL)
533-
xpfree(rowid);
552+
xpfree(rowid);
534553
}
535554

536-
xpfree(fctx->lastrowid);
555+
/*
556+
* switch to memory context appropriate for multiple function
557+
* calls
558+
*/
559+
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
537560

538-
if (values[0] != NULL)
539-
{
540-
/*
541-
* switch to memory context appropriate for multiple
542-
* function calls
543-
*/
544-
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
561+
xpfree(fctx->lastrowid);
562+
xpstrdup(fctx->lastrowid, values[0]);
563+
lastrowid = fctx->lastrowid;
545564

546-
lastrowid = fctx->lastrowid = pstrdup(values[0]);
547-
MemoryContextSwitchTo(oldcontext);
548-
}
565+
MemoryContextSwitchTo(oldcontext);
549566

550-
if (!allnulls)
567+
if (!skip_tuple)
551568
{
552569
/* build the tuple */
553570
tuple = BuildTupleFromCStrings(attinmeta, values);
@@ -566,8 +583,8 @@ crosstab(PG_FUNCTION_ARGS)
566583
else
567584
{
568585
/*
569-
* Skipping this tuple entirely, but we need to advance
570-
* the counter like the API would if we had returned one.
586+
* Skipping this tuple entirely, but we need to advance the
587+
* counter like the API would if we had returned one.
571588
*/
572589
call_cntr = ++funcctx->call_cntr;
573590

@@ -581,11 +598,14 @@ crosstab(PG_FUNCTION_ARGS)
581598
SPI_finish();
582599
SRF_RETURN_DONE(funcctx);
583600
}
601+
602+
/* need to reset this before the next tuple is started */
603+
skip_tuple = false;
584604
}
585605
}
586606
}
587607
else
588-
/* do when there is no more left */
608+
/* do when there is no more left */
589609
{
590610
/* release SPI related resources */
591611
SPI_finish();

0 commit comments

Comments
 (0)