Hi Amit,
> > > >> > >> 4. > >> +# Testcase start: SUBSCRIPTION BEHAVIOR WITH ENABLE_INDEXSCAN > >> > >> As we have removed enable_indexscan check, you should remove this test. > > > > > > Hmm, I think my rebase problems are causing confusion here, which v38 > fixes. > > > > I think it is still not fixed in v38 as the test is still present in 0001. > Ah, yes, sorry again for the noise. v39 will drop that. > > > In the first commit, we have ENABLE_INDEXSCAN checks. In the second > commit, > > I changed the same test to use enable_replica_identity_full_index_scan. > > > > If we are going to only consider the first patch to get into the master > branch, > > I can probably drop the test. In that case, I'm not sure what is our > perspective > > on ENABLE_INDEXSCAN GUC. Do we want to keep that guard in the code > > (hence the test)? > > > > I am not sure what we are going to do on this because I feel we need > some storage option as you have in 0002 patch but you and Andres > thinks that is not required. So, we can discuss that a bit more after > 0001 is committed but if there is no agreement then we need to > probably drop it. Even if drop it, I don't think using enable_index > makes sense. I think for now you don't need to send 0002 patch, let's > first try to get 0001 patch and then we can discuss about 0002. > sounds good, when needed I'll rebase 0002. > > > > Also, you have not noted, but I think SUBSCRIPTION USES INDEX WITH > MULTIPLE COLUMNS > > already covers SUBSCRIPTION USES INDEX UPDATEs MULTIPLE ROWS. > > > > So, I changed the first one to SUBSCRIPTION USES INDEX WITH MULTIPLE > ROWS AND COLUMNS > > and dropped the second one. Let me know if it does not make sense to > you. If I try, there are few more > > opportunities to squeeze in some more tests together, but those would > start to complicate readability. > > > > I still want to reduce the test time and will think about it. Which of > the other tests do you think can be combined? > > I'll follow your suggestion in the next e-mail [2], and focus on further improvements. BTW, did you consider updating the patch based on my yesterday's email [1]? > > Yes, replied to that one just now with some wip commits [1] > It appears you are using inconsistent spacing. It may be better to use > a single empty line wherever required. > > Sure, let me fix those. attached v39. [1] https://www.postgresql.org/message-id/CACawEhXGnk6v7UOHrxuJjjybHvvq33Zv666ouy4UzjPfJM6tBw%40mail.gmail.com [2] https://www.postgresql.org/message-id/CAA4eK1LSYWrthA3xjbrZvZVmwuha10HtM3-QRrVMD7YBt4t3pg%40mail.gmail.com
v39_0001_use_index_on_subs_when_pub_rep_ident_full.patch
Description: Binary data
