On 07.06.21 13:50, David Rowley wrote:
On Mon, 7 Jun 2021 at 23:46, Edmund Horner wrote:
On Mon, 7 Jun 2021 at 22:11, Peter Eisentraut
wrote:
This patch didn't add _outTidRangePath() even though we have outNode()
coverage for most/all path nodes. Was this just forgotten? See
attached
On Mon, 7 Jun 2021 at 23:46, Edmund Horner wrote:
>
> On Mon, 7 Jun 2021 at 22:11, Peter Eisentraut
> wrote:
>>
>> This patch didn't add _outTidRangePath() even though we have outNode()
>> coverage for most/all path nodes. Was this just forgotten? See
>> attached patch.
>
>
> Yes, it looks
On Mon, 7 Jun 2021 at 22:11, Peter Eisentraut <
peter.eisentr...@enterprisedb.com> wrote:
> This patch didn't add _outTidRangePath() even though we have outNode()
> coverage for most/all path nodes. Was this just forgotten? See
> attached patch.
>
Yes, it looks like an omission. Thanks for
This patch didn't add _outTidRangePath() even though we have outNode()
coverage for most/all path nodes. Was this just forgotten? See
attached patch.
From 3c696f812d4c6f8c66bc75105c3c1af79c3b2922 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 7 Jun 2021 12:04:49 +0200
Subject:
On Fri, 19 Feb 2021 at 20:37, David Rowley wrote:
>
> On Thu, 18 Feb 2021 at 09:45, David Rowley wrote:
> >
> > On Wed, 17 Feb 2021 at 11:05, Andres Freund wrote:
> > > How does this interact with rescans?
> >
> > We must call table_rescan() before calling table_set_tidrange() again.
> > That
On Thu, 18 Feb 2021 at 09:45, David Rowley wrote:
>
> On Wed, 17 Feb 2021 at 11:05, Andres Freund wrote:
> > Architecturally it feels like this is something that really belongs more
> > into plan time?
>
> Possibly. It would mean TidOpExpr would have to become a Node type.
> TID Range scan is
Thanks for having a look at this.
On Wed, 17 Feb 2021 at 11:05, Andres Freund wrote:
>
> On 2021-02-04 23:51:39 +1300, David Rowley wrote:
> > and
> > bool (*scan_tid_range_nextslot) (TableScanDesc sscan,
> > ScanDirection direction,
> > TupleTableSlot *slot);
> >
> > I added an additional
Hi,
On 2021-02-04 23:51:39 +1300, David Rowley wrote:
> I ended up adding just two new API functions to table AM.
>
> void (*scan_set_tid_range) (TableScanDesc sscan,
>ItemPointer mintid,
>ItemPointer maxtid);
>
> and
> bool (*scan_tid_range_nextslot) (TableScanDesc sscan,
>
On Tue, Feb 16, 2021 at 10:22:41PM +1300, David Rowley wrote:
> On Thu, 4 Feb 2021 at 23:51, David Rowley wrote:
> > Updated patch attached.
>
> I made another pass over this patch and did a bit of renaming work
> around the heap_* functions and the tableam functions. I think the new
> names are
On Thu, 4 Feb 2021 at 10:31, David Rowley wrote:
>
> Thanks for looking at this.
>
> On Thu, 4 Feb 2021 at 10:19, Andres Freund wrote:
> > Perhaps something like
> >
> > typedef struct TableScanTidRange TableScanTidRange;
> >
> > TableScanTidRange* table_scan_tid_range_start(TableScanDesc sscan,
Thanks for looking at this.
On Thu, 4 Feb 2021 at 10:19, Andres Freund wrote:
> Perhaps something like
>
> typedef struct TableScanTidRange TableScanTidRange;
>
> TableScanTidRange* table_scan_tid_range_start(TableScanDesc sscan,
> ItemPointer mintid, ItemPointer maxtid);
> bool
Hi,
On 2021-01-26 14:22:42 +1300, David Rowley wrote:
> diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
> index 33bffb6815..d1c608b176 100644
> --- a/src/include/access/tableam.h
> +++ b/src/include/access/tableam.h
> @@ -325,6 +325,26 @@ typedef struct TableAmRoutine
>
Thanks for having a look at this.
On Tue, 26 Jan 2021 at 15:48, Zhihong Yu wrote:
> bq. within this range. Table AMs where scanning ranges of TIDs does not make
> sense or is difficult to implement efficiently may choose to not implement
>
> Is there criterion on how to judge efficiency ?
For
Hi,
bq. within this range. Table AMs where scanning ranges of TIDs does not
make
sense or is difficult to implement efficiently may choose to not implement
Is there criterion on how to judge efficiency ?
+ if (tidopexpr->exprtype == TIDEXPR_LOWER_BOUND)
...
+ if
On Thu, 21 Jan 2021 at 18:16, David Rowley wrote:
> I've implemented this in the attached.
The bug fix in 0001 is now committed, so I'm just attaching the 0002
patch again after having rebased... This is mostly just to keep the
CFbot happy.
David
From e459b522d0599602188fcb1cc9ee6062ac8a4aee
On Wed, 13 Jan 2021 at 15:38, Edmund Horner wrote:
> Thanks for updating the patch. I'd be very happy if this got picked up
> again, and I'd certainly follow along and do some review.
Likewise here. I this patch was pretty close so it seems a shame to
let it slip through the cracks.
I spoke
On Fri, 1 Jan 2021 at 14:30, David Fetter wrote:
> On Sun, Dec 01, 2019 at 11:34:16AM +0900, Michael Paquier wrote:
> > Okay, still nothing has happened after two months. Once this is
> > solved a new patch submission could be done. For now I have marked
> > the entry as returned with
On Sun, Dec 01, 2019 at 11:34:16AM +0900, Michael Paquier wrote:
> On Thu, Sep 05, 2019 at 01:06:56PM +1200, Edmund Horner wrote:
> > So, I think we need to either get some help from someone familiar with
> > heapam.c, or maybe shelve the patch. It has been work in progress for
> > over a year
On Thu, Sep 05, 2019 at 01:06:56PM +1200, Edmund Horner wrote:
> So, I think we need to either get some help from someone familiar with
> heapam.c, or maybe shelve the patch. It has been work in progress for
> over a year now.
Okay, still nothing has happened after two months. Once this is
On Wed, 4 Sep 2019 at 10:34, Alvaro Herrera wrote:
>
> On 2019-Aug-01, Edmund Horner wrote:
>
> > Hi everyone,
> >
> > Sadly it is the end of the CF and I have not had much time to work on
> > this. I'll probably get some time in the next month to look at the
> > heapam changes.
> >
> > Should
On 2019-Aug-01, Edmund Horner wrote:
> Hi everyone,
>
> Sadly it is the end of the CF and I have not had much time to work on
> this. I'll probably get some time in the next month to look at the
> heapam changes.
>
> Should we move to CF? We have been in the CF cycle for almost a year now...
On Thu, 1 Aug 2019 at 17:57, Thomas Munro wrote:
>
> On Thu, Aug 1, 2019 at 5:51 PM Edmund Horner wrote:
> > I tried moving it to the new commitfest, but it seems I cannot from
> > its current state.
>
> Done. You have to move it to "Needs review" first, and then move to
> next. (Perhaps we
On Thu, Aug 1, 2019 at 5:51 PM Edmund Horner wrote:
> On Thu, 1 Aug 2019 at 17:47, Thomas Munro wrote:
> > On Thu, Aug 1, 2019 at 5:34 PM Edmund Horner wrote:
> > > Should we move to CF? We have been in the CF cycle for almost a year
> > > now...
> >
> > Hi Edmund,
> >
> > No worries, that's
On Thu, 1 Aug 2019 at 17:47, Thomas Munro wrote:
> On Thu, Aug 1, 2019 at 5:34 PM Edmund Horner wrote:
> > Should we move to CF? We have been in the CF cycle for almost a year now...
>
> Hi Edmund,
>
> No worries, that's how it goes sometimes. Please move it to the next
> CF if you think
On Thu, Aug 1, 2019 at 5:34 PM Edmund Horner wrote:
> Sadly it is the end of the CF and I have not had much time to work on
> this. I'll probably get some time in the next month to look at the
> heapam changes.
>
> Should we move to CF? We have been in the CF cycle for almost a year now...
Hi
On Mon, 22 Jul 2019 at 19:44, Edmund Horner wrote:
> On Mon, 22 Jul 2019 at 19:25, David Rowley
> > On Sat, 20 Jul 2019 at 06:21, Andres Freund wrote:
> > > Yea, I was thinking of something like 2. We already have a few extra
> > > types of scan nodes (bitmap heap and sample scan), it'd not be
On Mon, 22 Jul 2019 at 19:25, David Rowley
> On Sat, 20 Jul 2019 at 06:21, Andres Freund wrote:
> > Yea, I was thinking of something like 2. We already have a few extra
> > types of scan nodes (bitmap heap and sample scan), it'd not be bad to
> > add another. And as you say, they can just share
On Sat, 20 Jul 2019 at 06:21, Andres Freund wrote:
>
> Hi,
>
> On 2019-07-19 13:54:59 +1200, David Rowley wrote:
> > Did you imagined two additional callbacks, 1 to set the TID range,
> > then one to scan it? Duplicating the logic in heapgettup_pagemode()
> > and heapgettup() looks pretty
Hi,
On 2019-07-19 13:54:59 +1200, David Rowley wrote:
> On Thu, 18 Jul 2019 at 14:30, Andres Freund wrote:
> > I think the AM part of this patch might be the wrong approach - it won't
> > do anything meaningful for an AM that doesn't directly map the ctid to a
> > specific location in a block
On Thu, 18 Jul 2019 at 14:30, Andres Freund wrote:
> I think the AM part of this patch might be the wrong approach - it won't
> do anything meaningful for an AM that doesn't directly map the ctid to a
> specific location in a block (e.g. zedstore). To me it seems the
> callback ought to be to
Hi,
On 2019-07-17 23:10:52 +1200, David Rowley wrote:
> When I mentioned up-thread about the optional scan_setlimits table AM
> callback, I'd forgotten that you'd not have access to check that
> directly during planning. As you mention above, you've added
> RelOptInfo has_scan_setlimits so the
Thanks for the edits and fixing that pretty glaring copy-paste bug.
Regarding enable_tidscan, I couldn't decide whether we really need it,
and erred on the side of not adding yet another setting.
The current patch only creates a tid range path if there's at least
one ctid qual. But during
On Mon, 15 Jul 2019 at 17:54, Edmund Horner wrote:
> Summary of changes compared to last time:
> - I've added the additional "scan_setlimits" table AM method. To
> check whether it's implemented in the planner, I have added an
> additional "has_scan_setlimits" flag to RelOptInfo. It seems to
On Thu, 11 Jul 2019 at 10:22, David Rowley wrote:
> So it seems that the plan is to insist that TIDs are tuple identifiers
> for all table AMs, for now. If that changes in the future, then so be
> it, but I don't think that's cause for delaying any work on TID Range
> Scans. Also from scanning
On Thu, 11 Jul 2019 at 10:22, David Rowley wrote:
> > A. Continue to target only heapam tables, making the bare minimum
> > changes necessary for the new tableam api.
> > B. Try to do something more general that works on all tableam
> > implementations for which it may be useful.
>
> Going by
On Sun, 7 Jul 2019 at 15:32, Edmund Horner wrote:
> I'm not really sure how to proceed. I started with a fairly pragmatic
> solution to "WHERE ctid > ? AND ctid < ?" for tables, and then tableam
> came along.
>
> The options I see are:
>
> A. Continue to target only heapam tables, making the
On Thu, 4 Jul 2019 at 15:43, David Rowley wrote:
> On Mon, 1 Jul 2019 at 23:29, Thomas Munro wrote:
> > The new CF is here. I'm going through poking threads for submissions
> > that don't apply, but it sounds like this needs more than a rebase?
> > Perhaps this belongs in the next CF?
>
> 0001
On Mon, 1 Jul 2019 at 23:29, Thomas Munro wrote:
> The new CF is here. I'm going through poking threads for submissions
> that don't apply, but it sounds like this needs more than a rebase?
> Perhaps this belongs in the next CF?
0001 and 0004 of v7 got pushed in PG12. The CFbot will be trying
On Tue, Mar 26, 2019 at 7:25 PM David Steele wrote:
> On 3/26/19 8:11 AM, Edmund Horner wrote:
> > Should I set update CF app to a) set the target version to 13, and/or
> > move it to next commitfest?
>
> If you plan to continue working on it in this CF then you can just
> change the target to
Hi,
On 2019-03-26 19:11:13 +1300, Edmund Horner wrote:
> The changes in heapam.c were required for backward scan support, as
> used by ORDER BY ctid DESC and MAX(ctid); and also for FETCH LAST and
> FETCH PRIOR. I have removed the backward scans functionality from the
> current set of patches,
On 3/26/19 8:11 AM, Edmund Horner wrote:
On Tue, 26 Mar 2019 at 11:54, Tom Lane wrote:
Andres Freund writes:
I was kinda hoping to keep block numbers out of the "main" APIs, to
avoid assuming everything is BLCKSZ based. I don't have a particular
problem allowing an optional setscanlimits
On Tue, 26 Mar 2019 at 11:54, Tom Lane wrote:
>
> Andres Freund writes:
> > I was kinda hoping to keep block numbers out of the "main" APIs, to
> > avoid assuming everything is BLCKSZ based. I don't have a particular
> > problem allowing an optional setscanlimits type callback that works with
>
Andres Freund writes:
> I was kinda hoping to keep block numbers out of the "main" APIs, to
> avoid assuming everything is BLCKSZ based. I don't have a particular
> problem allowing an optional setscanlimits type callback that works with
> block numbers. The planner could check its presence and
Hi,
On 2019-03-18 22:35:05 +1300, David Rowley wrote:
> The commit message in 8586bf7ed mentions:
>
> > Subsequent commits will incrementally abstract table access
> > functionality to be routed through table access methods. That change
> > is too large to be reviewed & committed at once, so
Hi,
On 2019-03-15 18:42:40 +1300, Edmund Horner wrote:
> I've had to adapt it to use the table scan API. I've got it compiling
> and passing tests, but I'm uneasy about some things that still use the
> heapam API.
>
> 1. I call heap_setscanlimits as I'm not sure there is a tableam
> equivalent.
On 3/18/19 1:35 PM, David Rowley wrote:
On Fri, 15 Mar 2019 at 18:42, Edmund Horner wrote:
Subsequent commits will incrementally abstract table access
functionality to be routed through table access methods. That change
is too large to be reviewed & committed at once, so it'll be done
On Fri, 15 Mar 2019 at 18:42, Edmund Horner wrote:
> I've had to adapt it to use the table scan API. I've got it compiling
> and passing tests, but I'm uneasy about some things that still use the
> heapam API.
>
> 1. I call heap_setscanlimits as I'm not sure there is a tableam equivalent.
> 2.
On Thu, 14 Mar 2019 at 23:37, Edmund Horner wrote:
> On Thu, 14 Mar 2019 at 23:06, David Rowley
> wrote:
> > Just looking again, I think the block of code starting:
> >
> > + if (density > 0.0)
> >
> > needs a comment to mention what it's doing. Perhaps:
> >
> > + /*
> > + * Using the average
On Thu, 14 Mar 2019 at 23:06, David Rowley wrote:
> On Thu, 14 Mar 2019 at 21:12, Edmund Horner wrote:
> > I'm not sure how an unreasonable underestimation would occur here. If
> > you have a table bloated to say 10x its minimal size, the estimator
> > still assumes an even distribution of
On Thu, 14 Mar 2019 at 21:12, Edmund Horner wrote:
> I'm not sure how an unreasonable underestimation would occur here. If
> you have a table bloated to say 10x its minimal size, the estimator
> still assumes an even distribution of tuples (I don't think we can do
> much better than that). So
On Thu, 14 Mar 2019 at 16:46, David Rowley wrote:
>
> The only possible risk I can foresee is that it may be more likely we
> underestimate the selectivity and that causes something like a nested
> loop join due to the estimation being, say 1 row.
>
> It could happen in a case like:
>
> SELECT *
On Mon, 4 Feb 2019 at 18:37, Edmund Horner wrote:
> 2. v6-0002-Support-backward-scans-over-restricted-ranges-in-hea.patch
> 3. v6-0003-Support-range-quals-in-Tid-Scan.patch
> 4. v6-0004-TID-selectivity-reduce-the-density-of-the-last-page-.patch
These ones need a rebase.
--
David Rowley
On Mon, 4 Feb 2019 at 18:37, Edmund Horner wrote:
> 1. v6-0001-Add-selectivity-estimate-for-CTID-system-variables.patch
I think 0001 is good to go. It's a clear improvement over what we do today.
(t1 = 1 million row table with a single int column.)
Patched:
# explain (analyze, timing off)
On Sat, 19 Jan 2019 at 17:04, Edmund Horner wrote:
> On Sat, 19 Jan 2019 at 05:35, Tom Lane wrote:
>
>> Edmund Horner writes:
>> > My patch uses the same path type and executor for all extractable
>> tidquals.
>>
>> > This worked pretty well, but I am finding it difficult to reimplement
>> it
Hi,
On 2019-01-19 17:04:13 +1300, Edmund Horner wrote:
> On Sat, 19 Jan 2019 at 05:35, Tom Lane wrote:
>
> > Edmund Horner writes:
> > > My patch uses the same path type and executor for all extractable
> > tidquals.
> >
> > > This worked pretty well, but I am finding it difficult to
On Sat, 19 Jan 2019 at 05:35, Tom Lane wrote:
> Edmund Horner writes:
> > My patch uses the same path type and executor for all extractable
> tidquals.
>
> > This worked pretty well, but I am finding it difficult to reimplement it
> in
> > the new tidpath.c code.
>
> I didn't like that approach
Edmund Horner writes:
> My patch uses the same path type and executor for all extractable tidquals.
> This worked pretty well, but I am finding it difficult to reimplement it in
> the new tidpath.c code.
I didn't like that approach to begin with, and would suggest that you go
over to using a
Hi all,
I am a bit stuck and I think it's best to try to explain where.
I'm still rebasing the patches for the changes Tom made to support
parameterised TID paths for joins. While the addition of join support
itself does not really touch the same code, the modernisation -- in
particular,
Edmund Horner writes:
> On Sat, 22 Dec 2018 at 07:10, Tom Lane wrote:
>> I'm not entirely sure why you're bothering; surely nulltestsel is
>> unrelated to what this patch is about?
> I found that it made a difference with selectivity of range comparisons,
> because clauselist_selectivity tries
On Sat, 22 Dec 2018 at 07:10, Tom Lane wrote:
> BTW, with respect to this bit in 0001:
>
> @@ -1795,6 +1847,15 @@ nulltestsel(PlannerInfo *root, NullTestType
> nulltesttype, Node *arg,
> return (Selectivity) 0; /* keep compiler quiet */
> }
> }
> +else if
BTW, with respect to this bit in 0001:
@@ -1795,6 +1847,15 @@ nulltestsel(PlannerInfo *root, NullTestType
nulltesttype, Node *arg,
return (Selectivity) 0; /* keep compiler quiet */
}
}
+else if (vardata.var && IsA(vardata.var, Var) &&
+ ((Var *)
Edmund Horner writes:
> Ok. I think that will simplify things. So if I follow you correctly,
> we should do:
> 1. If has_useful_pathkeys is true: generate pathkeys (for CTID ASC),
> and use truncate_useless_pathkeys on them.
> 2. If we have tid quals or pathkeys, emit a TID scan path.
Check.
On Fri, 21 Dec 2018 at 16:31, Tom Lane wrote:
>
> Edmund Horner writes:
> > For the forward scan, I seem to recall, from your merge join example,
> > that it's useful to set the pathkeys even when there are no
> > query_pathkeys. We just have to unconditionally set them so that the
> > larger
Edmund Horner writes:
> For the forward scan, I seem to recall, from your merge join example,
> that it's useful to set the pathkeys even when there are no
> query_pathkeys. We just have to unconditionally set them so that the
> larger plan can make use of them.
No. Look at indxpath.c: it does
On Fri, 21 Dec 2018 at 13:25, David Rowley wrote:
> On Fri, 21 Dec 2018 at 13:09, Edmund Horner wrote:
> > On Fri, 21 Dec 2018 at 11:21, Tom Lane wrote:
> > > I'm having a hard time wrapping my mind around why you'd bother with
> > > backwards TID scans. The amount of code needed versus the
On Fri, 21 Dec 2018 at 13:09, Edmund Horner wrote:
> On Fri, 21 Dec 2018 at 11:21, Tom Lane wrote:
> > I'm having a hard time wrapping my mind around why you'd bother with
> > backwards TID scans. The amount of code needed versus the amount of
> > usefulness seems like a pretty bad cost/benefit
Hi,
On 2018-12-20 18:06:41 -0500, Tom Lane wrote:
> Andres Freund writes:
> > On 2018-12-20 17:21:07 -0500, Tom Lane wrote:
> >> I'm having a hard time wrapping my mind around why you'd bother with
> >> backwards TID scans.
>
> > I've not followed this thread, but wouldn't that be quite useful
Andres Freund writes:
> On 2018-12-20 17:21:07 -0500, Tom Lane wrote:
>> I'm having a hard time wrapping my mind around why you'd bother with
>> backwards TID scans.
> I've not followed this thread, but wouldn't that be quite useful to be
> able to move old tuples to free space earlier in the
Hi,
On 2018-12-20 17:21:07 -0500, Tom Lane wrote:
> Edmund Horner writes:
> > [ tid scan patches ]
>
> I'm having a hard time wrapping my mind around why you'd bother with
> backwards TID scans. The amount of code needed versus the amount of
> usefulness seems like a pretty bad cost/benefit
Edmund Horner writes:
> [ tid scan patches ]
I'm having a hard time wrapping my mind around why you'd bother with
backwards TID scans. The amount of code needed versus the amount of
usefulness seems like a pretty bad cost/benefit ratio, IMO. I can
see that there might be value in knowing that
Review of v5:
0001: looks good.
0002:
1. I don't think you need palloc0() here. palloc() looks like it would be fine.
if (tidRangeArray->ranges == NULL)
tidRangeArray->ranges = (TidRange *)
palloc0(tidRangeArray->numAllocated * sizeof(TidRange));
if that wasn't the case, then you'll need to
On Thu, 22 Nov 2018 at 20:41, David Rowley wrote:
> I've now had a look over the latest patches and I've found a few more
> things. Many of these are a bit nitpicky, but certainly not all. I
> also reviewed 0004 this time.
Whew! A lot more things to look at.
I've tried to address most of
On Sat, 24 Nov 2018 at 15:46, Tomas Vondra wrote:
> On 11/24/18 1:56 AM, Edmund Horner wrote:
> > On Fri, 23 Nov 2018 at 07:03, Tomas Vondra
> > wrote:
> >> On 11/22/18 8:41 AM, David Rowley wrote:
> >>> ...
> >>> 3. I'd rather see EnsureTidRangeSpace() keep doubling the size of the
> >>>
On 11/24/18 1:56 AM, Edmund Horner wrote:
> On Fri, 23 Nov 2018 at 07:03, Tomas Vondra
> wrote:
>> On 11/22/18 8:41 AM, David Rowley wrote:
>>> ...
>>> 3. I'd rather see EnsureTidRangeSpace() keep doubling the size of the
>>> allocation until it reaches the required size. See how
>>>
On Fri, 23 Nov 2018 at 07:03, Tomas Vondra wrote:
> On 11/22/18 8:41 AM, David Rowley wrote:
> > ...
> > 3. I'd rather see EnsureTidRangeSpace() keep doubling the size of the
> > allocation until it reaches the required size. See how
> > MakeSharedInvalidMessagesArray() does it. Doing it this
On 11/22/18 8:41 AM, David Rowley wrote:
>
...
3. I'd rather see EnsureTidRangeSpace() keep doubling the size of the
allocation until it reaches the required size. See how
MakeSharedInvalidMessagesArray() does it. Doing it this way ensures
we always have a power of two sized array which is
On Mon, 12 Nov 2018 at 17:35, Edmund Horner wrote:
> Hi, here's the new patch(s).
>
> Mostly the same, but trying to address your comments from earlier as
> well as clean up a few other things I noticed.
Thanks for making those changes.
I've now had a look over the latest patches and I've found
Hi, here's the new patch(s).
Mostly the same, but trying to address your comments from earlier as
well as clean up a few other things I noticed.
Cheers,
Edmund
On Fri, 9 Nov 2018 at 15:01, Edmund Horner wrote:
>
> On Tue, 6 Nov 2018 at 16:40, David Rowley
> wrote:
> > I've been looking over
On Tue, 6 Nov 2018 at 16:40, David Rowley wrote:
> I've been looking over 0001 to 0003. I ran out of steam before 0004.
Hi David, thanks for another big review with lots of improvements.
> I like the design of the new patch. From what I threw so far at the
> selectivity estimation code, it
On Tue, 6 Nov 2018 at 16:52, Alvaro Herrera wrote:
> On 2018-Nov-06, David Rowley wrote:
> > 14. we pass 'false' to what?
> >
> > + * save the tuple and the buffer returned to us by the access methods in
> > + * our scan tuple slot and return the slot. Note: we pass 'false' because
> > + *
On 2018-Nov-06, David Rowley wrote:
> 14. we pass 'false' to what?
>
> + * save the tuple and the buffer returned to us by the access methods in
> + * our scan tuple slot and return the slot. Note: we pass 'false' because
> + * tuples returned by heap_getnext() are pointers onto disk pages and
On 4 November 2018 at 17:20, Edmund Horner wrote:
> I have managed to split my changes into 4 patches:
>
> v3-0001-Add-selectivity-and-nullness-estimates-for-the-ItemP.patch
> v3-0002-Support-range-quals-in-Tid-Scan.patch
> v3-0003-Support-backward-scans-over-restricted-ranges-in-hea.patch
>
Hi all,
I have managed to split my changes into 4 patches:
v3-0001-Add-selectivity-and-nullness-estimates-for-the-ItemP.patch
v3-0002-Support-range-quals-in-Tid-Scan.patch
v3-0003-Support-backward-scans-over-restricted-ranges-in-hea.patch
v3-0004-Tid-Scan-results-are-ordered.patch
(1) is
On Wed, 3 Oct 2018 at 17:36, David Rowley wrote:
> I know commit fest is over, but I made a pass of this to hopefully
> provide a bit of guidance so that it's closer for the November 'fest.
Hi David. Thanks for the review. It's fairly thorough and you must
have put some time into it -- I
On 28 September 2018 at 18:13, Edmund Horner wrote:
> On Fri, 28 Sep 2018 at 17:02, Edmund Horner wrote:
>> I did run pgindent over it though. :)
>
> But I didn't check if it still applied to master. Sigh. Here's one that
> does.
I know commit fest is over, but I made a pass of this to
On Fri, 28 Sep 2018 at 17:02, Edmund Horner wrote:
> I did run pgindent over it though. :)
But I didn't check if it still applied to master. Sigh. Here's one that does.
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 3395445..e89343f 100644
---
On Wed, 19 Sep 2018 at 18:56, David Rowley wrote:
>
> On 19 September 2018 at 18:04, Edmund Horner wrote:
> > I have been generally following this approach (handling more kinds of
> > TID comparisons), and have found myself doing things like pairing up >
> > with <, estimating how much of a
On Mon, 17 Sep 2018 at 23:21, David Rowley wrote:
> On 15 August 2018 at 11:11, Edmund Horner wrote:
>> So we'd extend that to:
>> - Include in the OR-list "range" subquals of the form (ctid > ? AND
>> ctid < ?) (either side could be optional, and we have to deal with >=
>> and <= and having
On 15 August 2018 at 11:11, Edmund Horner wrote:
> So we'd extend that to:
> - Include in the OR-list "range" subquals of the form (ctid > ? AND
> ctid < ?) (either side could be optional, and we have to deal with >=
> and <= and having ctid on the rhs, etc.).
> - Cost the range subquals by
On 12 August 2018 at 20:07, David Rowley wrote:
>> Since range scan execution is rather different from the existing
>> TidScan execution, I ended up making a new plan type, TidRangeScan.
>> There is still only one TidPath, but it has an additional member that
>> describes which method to use.
>
>
On Sun, Aug 12, 2018 at 8:07 AM, David Rowley
wrote:
> Thanks for working on this. It will great to see improvements made in this
> area.
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 12 August 2018 at 14:29, Edmund Horner wrote:
> To scratch an itch, I have been working on teaching TidScan how to do
> range queries, i.e. those using >=, <, BETWEEN, etc. This means we
> can write, for instance,
>
> SELECT * FROM t WHERE ctid >= '(1000,0)' AND ctid < '(2000,0)';
I
92 matches
Mail list logo
