Re: [PROPOSAL] Use SnapshotAny in get_actual_variable_range

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Dmitriy Sarafannikov <dsarafannikov(at)yandex(dot)ru>
Subject: Re: [PROPOSAL] Use SnapshotAny in get_actual_variable_range
Date: 2017-08-18 06:50:39
Message-ID: 20170818065039.15349.16851.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, failed
Spec compliant: tested, failed
Documentation: tested, failed

Hi! I've looked into the patch.

There is not so much of the code. The code itself is pretty clear and well documented. I've found just one small typo "transactionn" in HeapTupleSatisfiesNonVacuumable() comments.

Chosen Snapshot type is not a solution to the author's problem at hand. Though implemented SnapshotNonVacuumable is closer to rational choice than currently used SnapshotDirty for range sketching. As far as I can see this is what is get_actual_variable_range() is used for, despite word "actual" in it's name.
The only place where get_actual_variable_range() is used for actual range is preprocessed-out in get_variable_range():
/*
* XXX It's very tempting to try to use the actual column min and max, if
* we can get them relatively-cheaply with an index probe. However, since
* this function is called many times during join planning, that could
* have unpleasant effects on planning speed. Need more investigation
* before enabling this.
*/
#ifdef NOT_USED
if (get_actual_variable_range(root, vardata, sortop, min, max))
return true;
#endif

I think it would be good if we could have something like "give me actual values, but only if it is on first index page", like conditional locks. But I think this patch is a step to right direction.

Best regards, Andrey Borodin.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2017-08-18 08:10:29 Re: Add support for tuple routing to foreign partitions
Previous Message Michael Paquier 2017-08-18 06:30:31 Re: recovery_target_time = 'now' is not an error but still impractical setting