Re: [PATCH] Add support for SAOP in the optimizer for partial index paths - Mailing list pgsql-hackers

The topic is new to me. So, I thought I would review it to learn something.

Here are some quick review comments:

0) You are top posting your emails/responses. You can find the details here - https://wiki.postgresql.org/wiki/Mailing_Lists#Email_etiquette_mechanics

1) Getting white space error when applying PATCH-1, PATCH-4, PATCH-5.

2) If you would like then both test cases changes can be merged into single patch. That is PATCH-2 & PATCH-3 as one.

3) Also, the fix (PATCH-4) and improvement(PATCH-5) that you have done can be directly be part of PATCH-1 itself, because they are changing your written function only. I think, no need to create separate patch for them. I think 1 code patch and 1 test patch should suffice.

4) Your if block of "if (to_remove != NULL)" has become dead code after applying PATCH-4. Where you removed "if (!clauseset.nonempty)" block which was assigning "to_remove". So, I think you should remove all its dead code.

5) While building "suitable_indexes" & "base_proof_clauses" (there are 2 such more in the patches) you have created a separate code block {} for it. I guess such style is rare in PG coding.

6) Should we use different name for #define MAX_SAOP_ARRAY_SIZE or make it common for both predtest.c & indxpath.c?

Thanks,

Nishant Sharma,