Postgres Pro
Downloads
Home   >   mailing lists

Thread: Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

From
Michail Nikolaev
Date:
Hello!

One more thing (maybe I missed it in the patch, but anyway) - should we
add some migration to ensure what old databases will get enabled=true by
default after upgrade?

Best regards,
Mikhail.

Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

From
Sami Imseih
Date:
> Should this not behave like if you drop (or create) an index
> during a prepared statement? I have not yet looked closely at
> this code to see what could be done.
>
> Regards,

I looked at this a bit more and ATExecEnableDisableIndex
needs some tweaks.

What should be getting invalidated in the heap relation
that the index is on and not the index relation as it is in
the current patch.

You can retrieve the heap relation oid
IndexGetRelation(indexOid, false) and the
CacheInvalidateRelcache should be on the heap relation.

The planner needs to only care about the heap relation
invalidation to re-plan across multiple executions of
a prepared statement.

There should be a test for this scenario as well.

Regards,

Sami

Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

From
Shayon Mukherjee
Date:

On Mon, Dec 30, 2024 at 3:48 PM Michail Nikolaev <michail.nikolaev@gmail.com> wrote:
Hello!

One more thing (maybe I missed it in the patch, but anyway) - should we
add some migration to ensure what old databases will get enabled=true by
default after upgrade?

Hi! 

Thank you! I tested this by manually upgrading (using pg_upgrade) from master to the build from the branch, which ensures that post-upgrade the column for indisenabled is true by default. I also backed it up with bool indisenabled BKI_DEFAULT(t); in pg_index.h. Additionally, I tested upgrading from an old data directory to the new one (both on this patch) to ensure indexes with DISABLE properties are carried over as well on the new data directory/upgrade. For reference the latest patch now is in [1].

Given this is working as expected, would we still need a migration step? (Let me know if I missed something ofc).

For reference here is the setup from my local testing (for reference)

rm -Rf /tmp/pg_data && rm -Rf /tmp/pg_data_new
./configure --prefix=/tmp/pg_install_old && make clean && make -j8 && make install

# Create and init old cluster
/tmp/pg_install_old/bin/initdb -D /tmp/pg_data
/tmp/pg_install_old/bin/pg_ctl -D /tmp/pg_data start

# Create test data
/tmp/pg_install_old/bin/createdb test
/tmp/pg_install_old/bin/psql test -c "CREATE TABLE foo (id int); CREATE INDEX idx_foo ON foo(id) DISABLE;"

# Stop old cluster
/tmp/pg_install_old/bin/pg_ctl -D /tmp/pg_data stop

# Switch branch and build new version
git checkout s/enable-disable-index
./configure --prefix=/tmp/pg_install_new && make clean && make -j8 && make install

# Create new cluster directory
/tmp/pg_install_new/bin/initdb -D /tmp/pg_data_new

# Now run upgrade with different binary locations
/tmp/pg_install_new/bin/pg_upgrade \
  -b /tmp/pg_install_old/bin \
  -B /tmp/pg_install_new/bin \
  -d /tmp/pg_data \
  -D /tmp/pg_data_new \
  -p 5432 \
  -P 5433

/tmp/pg_install_new/bin/pg_ctl -D /tmp/pg_data_new start
$ SELECT * FROM pg_index WHERE indexrelid = 'idx_foo'::regclass;

Thank you
Shayon

Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

From
Michail Nikolaev
Date:
Hello!

> Given this is working as expected, would we still need a migration step? 

No, it is clear now. Thanks for explaining.

Best regards,
Mikhail.

Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

From
Sami Imseih
Date:
+       This is the
+      default state for newly created indexes.

This is not needed in the ALTER INDEX docs, IMO.ss

+     <para>
+      Disable the specified index. A disabled index is not used for
queries, but it
+      is still updated when the underlying table data changes and will still be
+      used to enforce constraints (such as UNIQUE, or PRIMARY KEY constraints).
+      This can be useful for testing query performance with and
without specific
+      indexes. If performance degrades after disabling an index, it
can be easily
+      re-enabled using <literal>ENABLE</literal>. Before disabling,
it's recommended
+      to check
<structname>pg_stat_user_indexes</structname>.<structfield>idx_scan</structfield>
+      to identify potentially unused indexes.
+     </para>

This got me thinking if dropping the index is the only
use case we really care about. For example, you may want
to prevent an index that is enforcing a constraint from
being used by the planner, but you probably don't want to
drop it. In fact, I also think that you may want the index
from being used in one part of your application but could
potentially benefit other parts of your application. In that
case, I can see a GUC that allows you to force the use of a
an index that has been CREATED or ALTERED as DISABLED.
UNlike the GUC suggested earlier in the thread, this GUC
can simply be a boolean to allow the force usage of a
DISABLED index. FWIW, Oracle has a similar parameter called
OPTIMIZER_USE_INVISIBLE_INDEXES.

+        underlying table data changes. This can be useful when you
want to create
+        an index without immediately impacting query performance,
allowing you to

c/performance/planning ??

I have also been thinking about DISABLE as the keyword,
and I really don't like it. DISABLE indicates, at least ot me,
that the index is not available for either reads or writes.

Looking at other engines, Sqlserver uses DISABLE to drop
the index data, but keeps the index metadata around.

Oracle uses INVISIBLE and MariabDB uses IGNORABLE to
provide similar functionality to that being discussed here. I
find those keywords to be more appropriate for this purpose.

What about if we use HIDDEN instead of DISABLE as the keyword?

Regards,

Sami