Re: [PATCH] Planner support function for generate_subscripts()

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