> >> >> 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.
