> If the reason for the stats polling was only to know if some index is
> chosen or not, I was wondering if you can just convey the same
> information to the TAP test via some conveniently placed (DEBUG?)
> logging.
>
I had thought about it but didn't convince myself that it would be a
better approach because it would LOG a lot of messages for bulk
updates/deletes.
I'm also hesitant to add any log messages for testing purposes, especially
something like this one, where a single UPDATE on the source code
leads to an unbounded number of logs.
1.
+# subscriber gets the missing table information
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION tap_sub_rep_full REFRESH PUBLICATION");
This and the follow-on test was not required after we have removed
Dropped columns test.
Right, I kept it with the idea that we might get the dropped column changes
earlier, so that I can rebase and add the drop column ones.
But, sure, we can add that later to other tests.
2. Reduce the number of updates/deletes in the first test to two rows.
We don't have any particular reasons to have more tuples. Given the
time constraints, I don't have any objections to change this.
3. Removed the cases for dropping the index. This ensures that after
dropping the index on the table we switch to either an index scan (if
a new index is created) or to a sequence scan. It doesn't seem like a
very interesting case to me.
For that test, my goal was to ensure/show that the invalidation callback
is triggered after `DROP / CREATE INDEX` commands.
Can we always assume that this would never change? Because if this
behavior ever changes, the users would stuck with the wrong/old
index until VACUUM happens.
Apart from the above, I have removed the explicit setting of
'wal_retrieve_retry_interval = 1ms' as the same is not done for any
other subscription tests. I know setting wal_retrieve_retry_interval
avoids the launcher sometimes taking more time to launch apply worker
but it is better to be consistent
Hmm, I cannot remember why I added that. It was probably to make
poll_query_until/wait_for_catchup to happen faster.
But, running the test w/wout this setting, I cannot observe any noticeable
difference. So, probably fine to remove.
. See the changes in
changes_amit_1.patch, if you agree with the same then please include
them in the next version.
included all, but I'm not very sure to remove (3). If you think we have
coverage for that in other cases, I'm fine with that.
After doing the above, the test time on my machine is closer to what
other tests take which is ~5s.
Yes, same for me.
Thanks, attaching v46