| From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Improving the names generated for indexes on expressions |
| Date: | 2026-06-16 16:14:25 |
| Message-ID: | CA+TgmoY2WSX-XwAU6y0ScvWuzYijWJ3k7RXJvGep7FX7M+RoFA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Jun 4, 2026 at 6:31 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I spent some time trying to parse out why I don't like that
> suggestion, and eventually realized that it's this: I don't want
> the behavior of CREATE INDEX to be dependent on every last detail
> of ruleutils.c. I'm afraid that that'd introduce undesirable
> cross-version changes in the names selected for indexes. Now,
> considering this sort of change at all requires an assumption that
> we can get away with breaking any applications that are sensitive
> to that. But we only have to assume that we can get away with that
> once. If we get ruleutils.c involved then I foresee a steady drip
> drip drip of edge-case naming changes, and I think that will annoy
> people.
Hmm, I don't know. I suppose you could go back and see whether making
these changes in the back-branches would produce different regression
test changes than making them against the master branch.
> In any event, the cfbot has been nagging me that this patch needs
> a rebase, so here's v3. The only change from v2 is that there are
> some new test cases in indexing.sql that need adjustment.
...but whether you do that research or not, I still think this is a
very significant improvement. For those not wanting to open up the
patch:
- An index on ((a + 0)) now gets the name idxpart1_a_0_idx instead of
idxpart1_expr_idx.
- An index on ((a + b)) now gets the name idxpart_a_b_idx instead of
idxpart_expr_idx.
- An index on (abs(b)) now gets the name idxpart1_abs_b_idx instead of
idxpart1_abs_idx.
Personal taste certainly enters into the calculus here, but IMHO
calling everything BLAH_expr_idx because the topmost thing is some
kind of operator invocation is a really poor user experience, and even
for cases where the top-level thing is a function invocation,
including more than just the name of the top-level function in the
index name seems like a really significant improvement.
I think it would be good to press ahead with this. Admittedly, you
have gotten feedback from a limited number of people, but if you
commit this soon-ish, there will be lots of time to hear from people
who dislike it before we run out of time to rethink for v20. If the
consensus is that we revert it or change the algorithm, so be it, but
it doesn't seem like you will learn much more about what people think
without committing something.
--
Robert Haas
EDB: http://www.enterprisedb.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-06-16 16:20:12 | Re: Direction for test frameworks: Perl TAP vs. Python/pytest |
| Previous Message | Nathan Bossart | 2026-06-16 16:14:10 | Re: Fix --missing-stats-only false positive for partitioned expression indexes |