| From: | Nishant Sharma <nishant(dot)sharma(at)enterprisedb(dot)com> |
|---|---|
| To: | Jim Vanns <james(dot)vanns(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: [PATCH] Add support for SAOP in the optimizer for partial index paths |
| Date: | 2026-06-22 11:34:01 |
| Message-ID: | CADrsxdZma8PEhvab72kAGNTxstnbT11MN-rAAG-eJa83rL2uWw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Thanks Jim for the patches!
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,
EDB, Pune.
https://www.enterprisedb.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuya Kawata | 2026-06-22 11:34:12 | Re: [PATCH] doc: clarify pg_stat_lock.fastpath_exceeded scope |
| Previous Message | Antonin Houska | 2026-06-22 11:12:11 | Re: REPACK CONCURRENTLY fails on tables with generated columns |