1. In the function GetCheapestReplicaIdentityFullPath.
+ if (rel->pathlist == NIL)
+ {
+ /*
+ * A sequential scan could have been dominated by by an index scan
+ * during make_one_rel(). We should always have a sequential scan
+ * before set_cheapest().
+ */
+ Path *seqScanPath = create_seqscan_path(root, rel, NULL, 0);
+
+ add_path(rel, seqScanPath);
+ }
This is a question I'm not sure about:
Do we need this part to add sequential scan?
I think in our case, the sequential scan seems to have been added by the
function make_one_rel (see function set_plain_rel_pathlist).
Yes, the sequential scan is added during make_one_rel.
If I am missing
something, please let me know. BTW, there is a typo in above comment: `by by`.
As the comment mentions, the sequential scan could have been dominated & removed by index scan, see add_path():
> *We also remove from the rel's pathlist any old paths that are dominated
* by new_path --- that is, new_path is cheaper, at least as well ordered,
* generates no more rows, requires no outer rels not required by the old
* path, and is no less parallel-safe.
Still, I agree that the comment could be improved, which I pushed.
2. In the file execReplication.c.
+#ifdef USE_ASSERT_CHECKING
+#include "catalog/index.h"
+#endif
#include "commands/trigger.h"
#include "executor/executor.h"
#include "executor/nodeModifyTable.h"
#include "nodes/nodeFuncs.h"
#include "parser/parse_relation.h"
#include "parser/parsetree.h"
+#ifdef USE_ASSERT_CHECKING
+#include "replication/logicalrelation.h"
+#endif
I think it's fine to only add `logicalrelation.h` here, because `index.h` has
been added by `logicalrelation.h`.
Makes sense, removed thanks.
Attached v13.
