Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

Поиск
Список
Период
Сортировка
От Önder Kalacı
Тема Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
Дата
Msg-id CACawEhXJRjKpSy7bfqi2TYhii-3a-UvprFfuxUd+QzKE9x7p9w@mail.gmail.com
обсуждение исходный текст
Ответ на RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher  ("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>)
Список pgsql-hackers
Hi Hayato, Amit, all




```
+       /* Build scankey for every non-expression attribute in the index. */
```

I think that single line comments should not terminated by ".".

Hmm, looking into execReplication.c, many of the single line comments
terminated by ".".  Also, On the HEAD, the same comment has single
line comment. So, I'd rather stick to that?

 

```
+       /* There should always be at least one attribute for the index scan. */
```

Same as above.

Same as above :) 
 


```
+#ifdef USE_ASSERT_CHECKING
+                       IndexInfo  *indexInfo = BuildIndexInfo(idxrel);
+
+                       Assert(!IsIndexOnlyOnExpression(indexInfo));
+#endif
```

I may misunderstand, but the condition of usable index has alteady been checked
when the oid was set but anyway the you confirmed the condition again before
really using that, right?
So is it OK not to check another assumption that the index is b-tree, non-partial,
and one column reference?
IIUC we can do that by adding new function like IsIndexUsableForReplicaIdentityFull()
that checks these condition, and then call at RelationFindReplTupleByIndex() if
idxIsRelationIdentityOrPK is false.

I think adding a function like IsIndexUsableForReplicaIdentityFull is useful. I can use it inside
FindUsableIndexForReplicaIdentityFull() and assert here. Also good for readability.

So, I mainly moved this assert to a more generic place with a more generic check
to RelationFindReplTupleByIndex
 

032_subscribe_use_index.pl

```
+# Testcase start: SUBSCRIPTION CREATE/DROP INDEX WORKS WITHOUT ISSUES
...
+# Testcase end: SUBSCRIPTION RE-CALCULATES INDEX AFTER CREATE/DROP INDEX
```

There is still non-consistent case.


Fixed, thanks

Anyway, I see your point, so
if you want to keep the naming as you proposed at least don't change
the ordering for get_opclass_input_type() call because that looks odd
to me.

(A small comment from Amit's previous e-mail)

Sure, applied now.

Attaching v31.

Thanks for the reviews!
Onder KALACI
 
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Heikki Linnakangas
Дата:
Сообщение: Re: Testing autovacuum wraparound (including failsafe)
Следующее
От: Jelte Fennema
Дата:
Сообщение: Re: [EXTERNAL] Re: Support load balancing in libpq