On Tue, Apr 1, 2025 at 2:41 PM Sami Imseih wrote:
> I went back to look at this patch and a few things. I noticed it did
> not have correct
> indentation, so I ran pgindent. I also removed some extra lines added and
> made
> some slight adjustments to the docs. Attached my edited patch as a txt.
I went back to look at this patch and a few things. I noticed it did
not have correct
indentation, so I ran pgindent. I also removed some extra lines added and made
some slight adjustments to the docs. Attached my edited patch as a txt. If you
agree, please revise into a v14.
I also noticed that b
Hi Hackers,
Just leaving a quick note: I know this patch has through a lot of variations. I
am keen on shipping this for v18 if possible, and while it’s a learning process
for me, I am more than happy to iterate on any feedback. Let me know if there
is anything I can do to help to keep the mome
On Sun, Feb 23, 2025 at 3:41 PM Shayon Mukherjee wrote:
> I have rebased the patch on top of master (resolving some merge
> conflicts), along with the meson changes (thank you for that).
>
Rebased against the latest master and attaching the v13 patch.
Thank you
Shayon
v13-0001-Introduce-the-a
On Sat, Feb 8, 2025 at 12:41 AM jian he wrote:
> hi.
> ```
> drop table if exists idxpart;
> create table idxpart (a int, b int, c text) partition by range (a);
> create table idxpart1 (like idxpart);
> alter table idxpart attach partition idxpart1 for values from (0) to (10);
>
> create index id
hi.
```
drop table if exists idxpart;
create table idxpart (a int, b int, c text) partition by range (a);
create table idxpart1 (like idxpart);
alter table idxpart attach partition idxpart1 for values from (0) to (10);
create index idxpart_c on only idxpart (c) invisible;
create index idxpart1_c o
On Sun, Feb 2, 2025 at 3:11 PM jian he wrote:
> hi.
> the following reviews based on
> v10-0001-Introduce-the-ability-to-set-index-visibility-us.patch.
>
> Thank you for the amazing review!!
> in src/test/regress/sql/create_index.sql
> seems there are no sql tests for "create index ... invisibl
hi.
modules/test_ddl_deparse/test_ddl_deparse.so.p/test_ddl_deparse.c.o -c
../../Desktop/pg_src/src7/postgres/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
../../Desktop/pg_src/src7/postgres/src/test/modules/test_ddl_deparse/test_ddl_deparse.c:
In function ‘get_altertable_subcmdinfo’:
.
hi.
the following reviews based on
v10-0001-Introduce-the-ability-to-set-index-visibility-us.patch.
in src/test/regress/sql/create_index.sql
seems there are no sql tests for "create index ... invisible"?
VISIBLE
Make the specified index visible. The index will be used fo
On Fri, Jan 24, 2025 at 8:56 PM Benoit Lobréau
wrote:
> I also noticed that \d on an index doesn't warn about the invisible state
> whereas \d on a table does:
>
Thank you for the review + patch (v9-002) [1]. Your patch looks good to me.
I have not incorporated this in my v10 patch [2]. Mostly t
On Fri, Jan 24, 2025 at 4:03 PM Benoit Lobréau
wrote:
> I did notice something in the command prototype:
>
> +ALTER INDEX [ IF EXISTS ] class="parameter">name VISIBLE
> +ALTER INDEX [ IF EXISTS ] class="parameter">name INVISIBLE
>
> it would probably be better as:
>
> +ALTER INDEX [ IF EXISTS
On Fri, Jan 31, 2025 at 10:18 AM Sami Imseih wrote:
>
> What is being discussed here is different from what I can tell. This
> is referring
> to the index changing status ( visible/invisible ) and those changes being
> visible by another transaction.
>
>
+1. My vote would be to keep the behavior
> I had the "BEGIN; ALTER INDEX; EXPLAIN; ROLLBACK;" scenario in mind, but
> didn't realise we acquire an AccessExclusiveLock on the index. Therefore, it's
> not possible to change the visibility within a single transaction
> unless you don’t mind blocking all access to the relation.
>
> I read
I did some additional testing with this command within transactions.
I had the "BEGIN; ALTER INDEX; EXPLAIN; ROLLBACK;" scenario in mind, but
didn't realise we acquire an AccessExclusiveLock on the index. Therefore, it's
not possible to change the visibility within a single transaction
unless
I also noticed that \d on an index doesn't warn about the invisible state
whereas \d on a table does:
[local]:5444 postgres@postgres=# SELECT indexrelid::regclass,
indisvalid, indisvisible FROM pg_index WHERE indexrelid =
'repli_pkey'::regclass \gx
-[ RECORD 1 ]+---
indexrelid | repli_pk
On Fri, Jan 24, 2025 at 11:32 AM Benoit Lobréau
wrote:
> The completion for the INVISIBLE / VISIBLE keyword is missing in psql.
I think this should to the trick ?
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 81cbf10aa28..43ea8e55fd0 100644
--- a/src/bin/psql
Hi,
Thank you for the patch! I've had a need for this feature several times,
so I appreciate the work you’ve put into it.
I like the new name VISIBLE/INVISIBLE and the fact that it's a separate flag in
pg_index (it's easy to monitor).
I don’t feel qualified to provide an opinion on the code itse
On Sat, Jan 11, 2025 at 5:50 PM Sami Imseih wrote:
> Here is a use-case where the GUC may be useful. I can see a user
> wanting to try out the index before committing to using it across the
> board. They can create the index as invisible and force using
> it in a specific part of the application.
Thanks for the updates patch!
>> 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 th
On Fri, Jan 3, 2025 at 4:09 PM Sami Imseih wrote:
> + This is the
> + default state for newly created indexes.
>
> This is not needed in the ALTER INDEX docs, IMO.ss
>
> Updated and attached the patch.
> This got me thinking if dropping the index is the only
> use case we really care
+ This is the
+ default state for newly created indexes.
This is not needed in the ALTER INDEX docs, IMO.ss
+
+ 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
+
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.
On Mon, Dec 30, 2024 at 3:48 PM Michail Nikolaev
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 (
On Mon, Dec 30, 2024 at 2:56 PM Sami Imseih wrote:
> > Rebased with the latest master as well.
>
> Hi,
>
> This is a great, long needed feature. Thanks for doing this.
>
> I am late to this thread, but I took a look at the current patch
> and have some comments as I continue to look.
>
>
Thank yo
> 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
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.
> Rebased with the latest master as well.
Hi,
This is a great, long needed feature. Thanks for doing this.
I am late to this thread, but I took a look at the current patch
and have some comments as I continue to look.
1/
instead of
+ If true, the index is currently enabled and should be
On Mon, Dec 30, 2024 at 11:08 AM Michail Nikolaev <
michail.nikol...@gmail.com> wrote:
> Hello.
>
> A few comments on patch:
>
Thank you for the feedback.
>
> > + temporarily reducing the overhead of index maintenance
> > + during bulk data loading operations
>
>>
>
But tuples are still ins
Hello.
A few comments on patch:
> + temporarily reducing the overhead of index maintenance
> + during bulk data loading operations
>
But tuples are still inserted, where the difference come from?
> or verifying an index is not being used
> + before dropping it
Hm, it does not provide
On Fri, Dec 6, 2024 at 11:24 AM Shayon Mukherjee wrote:
> The patch is also rebased against the latest master and passing in CI.
> Would love to receive any further feedback on it.
>
>
Rebased the last patch against the latest master from today as a v5. No
other changes since last post.
Shayon
On Mon, Nov 25, 2024 at 6:19 PM David Rowley wrote:
> Another safer option could be to disallow the enable/disable ALTER
> TABLE if indcheckxmin is true. We do have and use
> ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE for these sorts of issues.
> There are other existing failure modes relating to i
On Tue, 26 Nov 2024 at 11:34, Shayon Mukherjee wrote:
> Thank you for the guidance and tips. I was wondering if updating in-place is
> preferable or not, since my first instinct was to avoid it. I did not notice
> any breakage last time, unless I was looking in the wrong place (possibly?).
> I
> On Nov 5, 2024, at 10:55 AM, Robert Haas wrote:
>
> On Thu, Oct 17, 2024 at 9:59 AM Shayon Mukherjee wrote:
>> My take away from whether or not an in-place update is needed on pg_index [1]
>>
>> - It’s unclear to me why it’s needed.
>> - I understand the xmin would get incremented when using
On Thu, Oct 17, 2024 at 9:59 AM Shayon Mukherjee wrote:
> My take away from whether or not an in-place update is needed on pg_index [1]
>
> - It’s unclear to me why it’s needed.
> - I understand the xmin would get incremented when using CatalogTupleUpdate
> to update indisenabled.
> - However, I
Interesting idea.
This patch needs a rebase.
On Thu, 17 Oct 2024 at 15:59, Shayon Mukherjee wrote:
>
>
> On Oct 16, 2024, at 6:01 PM, Shayon Mukherjee wrote:
>
> I'll take some time to think this through and familiarize myself with the
> new systable_inplace_update_begin. In the meantime, aside
On Oct 16, 2024, at 6:01 PM, Shayon Mukherjee wrote:
I'll take some time to think this through and familiarize myself with the
new systable_inplace_update_begin. In the meantime, aside from the in-place
updates on pg_index, I would love to learn any first impressions or
feedback on the patch folk
> On Oct 16, 2024, at 2:15 PM, Shayon Mukherjee wrote:
>
>
>> On Oct 16, 2024, at 12:19 PM, Shayon Mukherjee wrote:
>>
>> - ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the pg_index
>> catalog to protect against indcheckxmin [2] (older unrelated thread).
>
> Performing th
On Oct 16, 2024, at 12:19 PM, Shayon Mukherjee wrote:- ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the pg_index catalog to protect against indcheckxmin [2] (older unrelated thread).Performing the in place update of the pg_index row from ATExecEnableDisableIndex using systable_i
On Oct 15, 2024, at 7:25 PM, David Rowley wrote:On Wed, 16 Oct 2024 at 03:40, Robert Haas wrote:On Sat, Oct 12, 2024 at 5:56 AM Shayon Mukherjee wrote:Thank you for sharing this Robert. I like the idea behind "allowing extensions to control planner behavior” overall and I think it does help towa
39 matches
Mail list logo
