Hi Priyanka, Thanks for the review, and for testing the patch yourself. v2 is attached.
1) This is already in the docs. The text below the function table in func-srf.sgml says: "Zero rows are returned for arrays that do not have the requested dimension, or if any input is NULL." The entries in the table itself are kept to a line or two, so I left it where it is. 2) I set arg3 to NULL. It is the only one that is not always given a value. arg1 and arg2 are set on the next line, so setting them to NULL first would be a wasted store. I did join their declarations and their assignments, as you suggest in 4). 3) I left that comment as it is. The same comment sits on the same test in six other support functions, so it is how we usually write it. 4) I kept those two lines apart. As one line it would be 87 columns wide, which is too long. The comment also makes more sense right above the assignment than above the declaration. 5) Added. I used a 3-D array where all three dimensions have a different length, so a mistake in the dimension indexing could not still print the right number. I used one case instead of four, since they all run the same line of code. While there I dropped one of the old cases: two of them checked the same rule, that we give up for a dimension above the first when the array is not a constant. 6) You are right. I think the reason is even better than the one you give. prorows is only used when the support function gives up. array_unnest_support() never gives up: it sets req->rows every time it is asked. So unnest() never uses its prorows of 100. For an array it knows nothing about, unnest() really estimates 10, which is the default in estimate_array_length(). My reason for picking 100 was to match unnest(), but I was matching a number that is never read. v2-0002 uses 10 instead. Two cases still fall back on prorows, and 10 fits both: * The dimension number is not known when we plan. At run time it is almost always 1, and 10 is what the dimension-1 case gives for an array with no statistics. * A dimension above the first is asked for on an array that is not a constant. One dimension of an array is never longer than the whole array, so a large number is wrong here. 1000 fits neither case. I kept 0002 as its own patch. It only changes a number in the catalog, and 0001 does not need it. If people do not agree on the number, 0001 can still go in. Thanks, Shihao
v2-0002-Lower-generate_subscripts-s-prorows-estimate-to-1.patch
Description: Binary data
v2-0001-Add-a-planner-support-function-for-generate_subsc.patch
Description: Binary data
