Re: Pluggable Storage - Andres's take

2019-06-06 Thread Alvaro Herrera
On 2019-Jun-06, Heikki Linnakangas wrote: > Pushed the first patch now. Andres already fixed the second issue in commit > b8b94ea129. Please don't omit the "Discussion:" tag in commit messages. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support,

Re: Pluggable Storage - Andres's take

2019-06-06 Thread Heikki Linnakangas
On 18/05/2019 01:19, Ashwin Agrawal wrote: On Tue, Apr 9, 2019 at 6:17 AM Heikki Linnakangas > wrote: On 08/04/2019 20:37, Andres Freund wrote: > On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: >> There's a little bug in index-only scan executor

Re: Pluggable Storage - Andres's take

2019-05-17 Thread Andres Freund
Hi, On 2019-05-17 14:49:19 -0700, Ashwin Agrawal wrote: > On Fri, May 17, 2019 at 12:54 PM Andres Freund wrote: > > > Hi, > > > > On 2019-05-15 23:00:38 -0700, Ashwin Agrawal wrote: > > > Highlevel this looks good to me. Will look into full details tomorrow. > > > > Ping? > > > > I'll push the

Re: Pluggable Storage - Andres's take

2019-05-17 Thread Andres Freund
Hi, On 2019-05-17 16:56:04 -0700, Ashwin Agrawal wrote: > Question on the patch, if not too late > Why call table_beginscan() in TidNext() and not in ExecInitTidScan() ? > Seems cleaner to have it in ExecInitTidScan(). Largely because it's symmetrical to where most other scans are started ( c.f.

Re: Pluggable Storage - Andres's take

2019-05-17 Thread Ashwin Agrawal
On Wed, May 15, 2019 at 11:54 AM Andres Freund wrote: > Attached is a prototype of a variation of this. I added a > table_tuple_tid_valid(TableScanDesc sscan, ItemPointer tid) > callback / wrapper. Currently it just takes a "plain" scan, but we could > add a separate table_beginscan variant too.

Re: Pluggable Storage - Andres's take

2019-05-17 Thread Ashwin Agrawal
On Tue, Apr 9, 2019 at 6:17 AM Heikki Linnakangas wrote: > On 08/04/2019 20:37, Andres Freund wrote: > > On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: > >> There's a little bug in index-only scan executor node, where it mixes > up the > >> slots to hold a tuple from the index, and from

Re: Pluggable Storage - Andres's take

2019-05-17 Thread Ashwin Agrawal
On Fri, May 17, 2019 at 12:54 PM Andres Freund wrote: > Hi, > > On 2019-05-15 23:00:38 -0700, Ashwin Agrawal wrote: > > Highlevel this looks good to me. Will look into full details tomorrow. > > Ping? > > I'll push the first of the patches soon, and unless you'll comment on > the second soon,

Re: Pluggable Storage - Andres's take

2019-05-17 Thread Andres Freund
Hi, On 2019-05-15 23:00:38 -0700, Ashwin Agrawal wrote: > Highlevel this looks good to me. Will look into full details tomorrow. Ping? I'll push the first of the patches soon, and unless you'll comment on the second soon, I'll also push ahead. There's a beta upcoming... - Andres

Re: Pluggable Storage - Andres's take

2019-05-16 Thread Ashwin Agrawal
On Wed, May 15, 2019 at 11:54 AM Andres Freund wrote: > Hi, > > On 2019-04-25 15:43:15 -0700, Andres Freund wrote: > > > > 3) nodeTidscan, skipping over too large tids > > >I think this should just be moved into the AMs, there's no need to > > >have this in nodeTidscan.c > > > > I think

Re: Pluggable Storage - Andres's take

2019-05-15 Thread Andres Freund
Hi, On 2019-04-25 15:43:15 -0700, Andres Freund wrote: > Hm. I think some of those changes would be a bit bigger than I initially > though. Attached is a more minimal fix that'd route > RelationGetNumberOfBlocksForFork() through tableam if necessary. I > think it's definitely the right answer

Re: Pluggable Storage - Andres's take

2019-05-09 Thread Ashwin Agrawal
On Wed, May 8, 2019 at 2:46 PM Andres Freund wrote: > > Hi, > > On 2019-05-07 23:18:39 -0700, Ashwin Agrawal wrote: > > On Mon, May 6, 2019 at 1:39 PM Ashwin Agrawal wrote: > > > Also wish to point out, while working on Zedstore, we realized that > > > TupleDesc from Relation object can be

Re: Pluggable Storage - Andres's take

2019-05-08 Thread Andres Freund
Hi, On 2019-05-07 23:18:39 -0700, Ashwin Agrawal wrote: > On Mon, May 6, 2019 at 1:39 PM Ashwin Agrawal wrote: > > Also wish to point out, while working on Zedstore, we realized that > > TupleDesc from Relation object can be trusted at AM layer for > > scan_begin() API. As for ALTER TABLE

Re: Pluggable Storage - Andres's take

2019-05-08 Thread Andres Freund
Hi, On 2019-04-29 16:17:41 -0700, Ashwin Agrawal wrote: > On Thu, Apr 25, 2019 at 3:43 PM Andres Freund wrote: > > Hm. I think some of those changes would be a bit bigger than I initially > > though. Attached is a more minimal fix that'd route > > RelationGetNumberOfBlocksForFork() through

Re: Pluggable Storage - Andres's take

2019-05-08 Thread Ashwin Agrawal
On Mon, May 6, 2019 at 1:39 PM Ashwin Agrawal wrote: > > Also wish to point out, while working on Zedstore, we realized that > TupleDesc from Relation object can be trusted at AM layer for > scan_begin() API. As for ALTER TABLE rewrite case (ATRewriteTables()), > catalog is updated first and

Re: Pluggable Storage - Andres's take

2019-05-07 Thread Rafia Sabih
On Mon, 6 May 2019 at 22:39, Ashwin Agrawal wrote: > > On Mon, May 6, 2019 at 7:14 AM Andres Freund wrote: > > > > Hi, > > > > On May 6, 2019 3:40:55 AM PDT, Rafia Sabih > > wrote: > > >I was trying the toyam patch and on make check it failed with > > >segmentation fault at > > > > > >static

Re: Pluggable Storage - Andres's take

2019-05-07 Thread Rafia Sabih
On Mon, 6 May 2019 at 16:14, Andres Freund wrote: > > Hi, > > On May 6, 2019 3:40:55 AM PDT, Rafia Sabih wrote: > >On Tue, 9 Apr 2019 at 15:17, Heikki Linnakangas > >wrote: > >> > >> On 08/04/2019 20:37, Andres Freund wrote: > >> > On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: > >> >>

Re: Pluggable Storage - Andres's take

2019-05-06 Thread Ashwin Agrawal
On Mon, May 6, 2019 at 7:14 AM Andres Freund wrote: > > Hi, > > On May 6, 2019 3:40:55 AM PDT, Rafia Sabih wrote: > >I was trying the toyam patch and on make check it failed with > >segmentation fault at > > > >static void > >toyam_relation_set_new_filenode(Relation rel, > > char persistence, >

Re: Pluggable Storage - Andres's take

2019-05-06 Thread Andres Freund
Hi, On May 6, 2019 3:40:55 AM PDT, Rafia Sabih wrote: >On Tue, 9 Apr 2019 at 15:17, Heikki Linnakangas >wrote: >> >> On 08/04/2019 20:37, Andres Freund wrote: >> > On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: >> >> There's a little bug in index-only scan executor node, where it

Re: Pluggable Storage - Andres's take

2019-05-06 Thread Rafia Sabih
On Tue, 9 Apr 2019 at 15:17, Heikki Linnakangas wrote: > > On 08/04/2019 20:37, Andres Freund wrote: > > On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: > >> There's a little bug in index-only scan executor node, where it mixes up > >> the > >> slots to hold a tuple from the index, and

Re: Pluggable Storage - Andres's take

2019-04-29 Thread Ashwin Agrawal
On Thu, Apr 25, 2019 at 3:43 PM Andres Freund wrote: > Hm. I think some of those changes would be a bit bigger than I initially > though. Attached is a more minimal fix that'd route > RelationGetNumberOfBlocksForFork() through tableam if necessary. I > think it's definitely the right answer for

Re: Pluggable Storage - Andres's take

2019-04-25 Thread Andres Freund
Hi Heikki, Ashwin, Tom, On 2019-04-23 15:52:01 -0700, Andres Freund wrote: > On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: > > index_update_stats() calls RelationGetNumberOfBlocks(). If the AM > > doesn't use normal data files, that won't work. I bumped into that with my > > toy

Re: Pluggable Storage - Andres's take

2019-04-24 Thread Robert Haas
On Tue, Apr 23, 2019 at 6:55 PM Tom Lane wrote: > Andres Freund writes: > > ... I think none of these are critical issues for tableam, but we should fix > > them. > > > I'm not sure about doing so for v12 though. 1) and 3) are fairly > > trivial, but 2) would involve changing the FDW interface,

Re: Pluggable Storage - Andres's take

2019-04-23 Thread Tom Lane
Andres Freund writes: > ... I think none of these are critical issues for tableam, but we should fix > them. > I'm not sure about doing so for v12 though. 1) and 3) are fairly > trivial, but 2) would involve changing the FDW interface, by changing > the AnalyzeForeignTable, AcquireSampleRowsFunc

Re: Pluggable Storage - Andres's take

2019-04-23 Thread Andres Freund
Hi, On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: > index_update_stats() calls RelationGetNumberOfBlocks(). If the AM > doesn't use normal data files, that won't work. I bumped into that with my > toy implementation, which wouldn't need to create any data files, if it > wasn't for this.

Re: Pluggable Storage - Andres's take

2019-04-18 Thread Andres Freund
Hi, On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: > The comments for relation_set_new_relfilenode() callback say that the AM can > set *freezeXid and *minmulti to invalid. But when I did that, VACUUM hits > this assertion: > > TRAP: FailedAssertion("!(((classForm->relfrozenxid) >=

Re: Pluggable Storage - Andres's take

2019-04-11 Thread Tom Lane
Andres Freund writes: > On 2019-04-11 14:52:40 +0300, Heikki Linnakangas wrote: >> + * HEIKKI: A flags bitmask argument would be more readable than 6 >> booleans > I honestly don't have strong feelings about it. Not sure that I buy that > bitmasks would be much more readable Sure they

Re: Pluggable Storage - Andres's take

2019-04-11 Thread Robert Haas
On Thu, Apr 11, 2019 at 12:49 PM Andres Freund wrote: > > @@ -179,6 +184,12 @@ typedef struct TableAmRoutine > >* > >* if temp_snap is true, the snapshot will need to be deallocated at > >* scan_end. > > + * > > + * HEIKKI: table_scan_update_snapshot() changes

Re: Pluggable Storage - Andres's take

2019-04-11 Thread Andres Freund
Hi, On 2019-04-11 14:52:40 +0300, Heikki Linnakangas wrote: > Here is another iteration on the comments. The patch is a mix of > copy-editing and questions. The questions are marked with "HEIKKI:". I can > continue the copy-editing, if you can reply to the questions, clarifying the > intention on

Re: Pluggable Storage - Andres's take

2019-04-11 Thread Heikki Linnakangas
On 08/04/2019 20:37, Andres Freund wrote: Hi, On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: There were a bunch of typos in the comments in tableam.h, see attached. Some of the comments could use more copy-editing and clarification, I think, but I stuck to fixing just typos and such

Re: Pluggable Storage - Andres's take

2019-04-09 Thread Heikki Linnakangas
On 08/04/2019 20:37, Andres Freund wrote: On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: There's a little bug in index-only scan executor node, where it mixes up the slots to hold a tuple from the index, and from the table. That doesn't cause any ill effects if the AM uses

Re: Pluggable Storage - Andres's take

2019-04-08 Thread Andres Freund
Hi, On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: > There were a bunch of typos in the comments in tableam.h, see attached. Some > of the comments could use more copy-editing and clarification, I think, but > I stuck to fixing just typos and such for now. I pushed these after adding

Re: Pluggable Storage - Andres's take

2019-04-08 Thread Fabrízio de Royes Mello
On Mon, Apr 8, 2019 at 9:34 AM Heikki Linnakangas wrote: > > I wrote a little toy implementation that just returns constant data to > play with this a little. Looks good overall. > > There were a bunch of typos in the comments in tableam.h, see attached. > Some of the comments could use more

Re: Pluggable Storage - Andres's take

2019-04-08 Thread Heikki Linnakangas
I wrote a little toy implementation that just returns constant data to play with this a little. Looks good overall. There were a bunch of typos in the comments in tableam.h, see attached. Some of the comments could use more copy-editing and clarification, I think, but I stuck to fixing just

Re: Pluggable Storage - Andres's take

2019-04-05 Thread Andres Freund
Hi, On 2019-04-04 00:51:38 -0500, Justin Pryzby wrote: > I reviewed new docs for $SUBJECT. > Find attached proposed changes. > There's one XXX item I'm unsure what it's intended to say. Thanks! I applied most of these, and filled in the XXX. I didn't like the s/allow to to specify

Re: Pluggable Storage - Andres's take

2019-04-03 Thread Justin Pryzby
I reviewed new docs for $SUBJECT. Find attached proposed changes. There's one XXX item I'm unsure what it's intended to say. Justin >From a3d290bf67af2a34e44cd6c160daf552b56a13b5 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Thu, 4 Apr 2019 00:48:09 -0500 Subject: [PATCH v1] Fine tune

Re: Pluggable Storage - Andres's take

2019-04-02 Thread Andres Freund
Hi, On 2019-04-02 17:11:07 +1100, Haribabu Kommi wrote: > From a72cfcd523887f1220473231d7982928acc23684 Mon Sep 17 00:00:00 2001 > From: Hari Babu > Date: Tue, 2 Apr 2019 15:41:17 +1100 > Subject: [PATCH 1/2] tableam : doc update of table access methods > > Providing basic explanation of table

Re: Pluggable Storage - Andres's take

2019-04-02 Thread Andres Freund
Hi, On 2019-04-02 17:11:07 +1100, Haribabu Kommi wrote: > + xreflabel="default_table_access_method"> > + default_table_access_method > (string) > + > + default_table_access_method configuration > parameter > + > + > + > + > +The value is

Re: Pluggable Storage - Andres's take

2019-04-02 Thread Haribabu Kommi
On Tue, Apr 2, 2019 at 11:53 AM Andres Freund wrote: > Hi, > > On 2019-04-02 11:39:57 +1100, Haribabu Kommi wrote: > > > What made you rename indexam.sgml to am.sgml, instead of creating a > > > separate tableam.sgml? Seems easier to just have a separate file? > > > > > > > No specific reason,

Re: Pluggable Storage - Andres's take

2019-04-01 Thread Andres Freund
Hi, On 2019-04-02 11:39:57 +1100, Haribabu Kommi wrote: > > What made you rename indexam.sgml to am.sgml, instead of creating a > > separate tableam.sgml? Seems easier to just have a separate file? > > > > No specific reason, I just thought of adding all the access methods under > one file. > I

Re: Pluggable Storage - Andres's take

2019-04-01 Thread Haribabu Kommi
On Tue, Apr 2, 2019 at 10:18 AM Andres Freund wrote: > Hi, > > On 2019-03-16 23:21:31 +1100, Haribabu Kommi wrote: > > updated patches are attached. > > Now that nearly all of the tableam patches are committed (with the > exception of the copy.c changes which are for bad reasons discussed at >

Re: Pluggable Storage - Andres's take

2019-04-01 Thread Andres Freund
Hi, On 2019-03-16 23:21:31 +1100, Haribabu Kommi wrote: > updated patches are attached. Now that nearly all of the tableam patches are committed (with the exception of the copy.c changes which are for bad reasons discussed at [1]) I'm looking at the docs changes. What made you rename

Re: Pluggable Storage - Andres's take

2019-03-29 Thread Andres Freund
On 2019-03-29 18:38:46 +1100, Haribabu Kommi wrote: > As I see that your are fixing some typos of the code that is committed, > I just want to share some more corrections that I found in the patches > that are committed till now. Pushed both, thanks!

Re: Pluggable Storage - Andres's take

2019-03-29 Thread Haribabu Kommi
On Wed, Mar 27, 2019 at 11:17 AM Andres Freund wrote: > Hi, > > On 2019-02-22 14:52:08 -0500, Robert Haas wrote: > > On Fri, Feb 22, 2019 at 11:19 AM Amit Khandekar > wrote: > > > Thanks for the review. Attached v2. > > > > Thanks. I took this, combined it with Andres's > >

Re: Pluggable Storage - Andres's take

2019-03-26 Thread Andres Freund
Hi, On 2019-02-22 14:52:08 -0500, Robert Haas wrote: > On Fri, Feb 22, 2019 at 11:19 AM Amit Khandekar > wrote: > > Thanks for the review. Attached v2. > > Thanks. I took this, combined it with Andres's > v12-0040-WIP-Move-xid-horizon-computation-for-page-level-.patch, did > some polishing of

Re: Pluggable Storage - Andres's take

2019-03-23 Thread Andres Freund
Hi, On 2019-03-23 20:16:30 -0700, Andres Freund wrote: > I'm pretty happy with that last version (of the first patch). I'm > planning to do one more pass, and then push. And done, after a bunch of mostly cosmetic changes (renaming ExecCheckHeapTupleVisible to ExecCheckTupleVisible, removing an

Re: Pluggable Storage - Andres's take

2019-03-23 Thread Andres Freund
Hi, On 2019-03-21 11:15:57 -0700, Andres Freund wrote: > Pending work: > - Wondering if table_insert/delete/update should rather be > table_tuple_insert etc. Would be a bit more consistent with the > callback names, but a bigger departure from existing code. I've left this as is. > - I'm

Re: Pluggable Storage - Andres's take

2019-03-21 Thread Haribabu Kommi
On Fri, Mar 22, 2019 at 5:16 AM Andres Freund wrote: > Hi, > > Attached is a version of just the first patch. I'm still updating it, > but it's getting closer to commit: > > - There were no tests testing EPQ interactions with DELETE, and only an > accidental test for EPQ in UPDATE with a

Re: Pluggable Storage - Andres's take

2019-03-20 Thread Haribabu Kommi
Hi, The psql \dA commands currently doesn't show the type of the access methods of type 'Table'. postgres=# \dA heap List of access methods Name | Type --+--- heap | (1 row) Attached a simple patch that fixes the problem and outputs as follows. postgres=# \dA heap List of access

Re: Pluggable Storage - Andres's take

2019-03-20 Thread Haribabu Kommi
On Sat, Mar 16, 2019 at 5:43 PM Haribabu Kommi wrote: > > > On Sat, Mar 9, 2019 at 2:13 PM Andres Freund wrote: > >> Hi, >> >> While 0001 is pretty bulky, the interesting bits concentrate on a >> comparatively small area. I'd appreciate if somebody could give the >> comments added in tableam.h

Re: Pluggable Storage - Andres's take

2019-03-16 Thread Haribabu Kommi
On Sat, Mar 9, 2019 at 2:13 PM Andres Freund wrote: > Hi, > > While 0001 is pretty bulky, the interesting bits concentrate on a > comparatively small area. I'd appreciate if somebody could give the > comments added in tableam.h a read (both on callbacks, and their > wrappers, as they have

Re: Pluggable Storage - Andres's take

2019-03-12 Thread Kyotaro HORIGUCHI
Hello. I had a look on the patch set. I cannot see the thread structure due to the depth and cannot get the picture on the all patches, but I have some comments. I apologize in advance for possible duplicate with upthread. 0001-Reduce-the... This doesn't apply master. > TupleTableSlot * >

Re: Pluggable Storage - Andres's take

2019-03-11 Thread Andres Freund
On 2019-03-11 13:31:26 -0700, Andres Freund wrote: > On 2019-03-11 12:37:46 -0700, Andres Freund wrote: > > Hi, > > > > On 2019-03-08 19:13:10 -0800, Andres Freund wrote: > > > Changes: > > > - I've added comments to all the callbacks in the first commit / the > > > scan commit > > > - I've

Re: Pluggable Storage - Andres's take

2019-03-11 Thread Andres Freund
On 2019-03-11 12:37:46 -0700, Andres Freund wrote: > Hi, > > On 2019-03-08 19:13:10 -0800, Andres Freund wrote: > > Changes: > > - I've added comments to all the callbacks in the first commit / the > > scan commit > > - I've renamed table_gimmegimmeslot to table_slot_create > > - I've made the

Re: Pluggable Storage - Andres's take

2019-03-10 Thread Haribabu Kommi
On Sat, Mar 9, 2019 at 2:18 PM Andres Freund wrote: > Hi, > > On 2019-03-09 11:03:21 +1100, Haribabu Kommi wrote: > > Here I attached the rebased patches that I shared earlier. I am adding > the > > comments to explain the API's in the code, will share those patches > later. > > I've started to

Re: Pluggable Storage - Andres's take

2019-03-09 Thread Andres Freund
Hi, On 2019-03-10 05:49:26 +0100, Dmitry Dolgov wrote: > > On Sat, Mar 9, 2019 at 4:13 AM Andres Freund wrote: > > > > While 0001 is pretty bulky, the interesting bits concentrate on a > > comparatively small area. I'd appreciate if somebody could give the > > comments added in tableam.h a read

Re: Pluggable Storage - Andres's take

2019-03-09 Thread Dmitry Dolgov
> On Sat, Mar 9, 2019 at 4:13 AM Andres Freund wrote: > > While 0001 is pretty bulky, the interesting bits concentrate on a > comparatively small area. I'd appreciate if somebody could give the > comments added in tableam.h a read (both on callbacks, and their > wrappers, as they have different

Re: Pluggable Storage - Andres's take

2019-03-08 Thread Andres Freund
Hi, On 2019-03-09 11:03:21 +1100, Haribabu Kommi wrote: > Here I attached the rebased patches that I shared earlier. I am adding the > comments to explain the API's in the code, will share those patches later. I've started to add those for the callbacks in the first commit. I'd appreciate a

Re: Pluggable Storage - Andres's take

2019-03-08 Thread Haribabu Kommi
On Thu, Mar 7, 2019 at 6:33 AM Andres Freund wrote: > Hi, > > On 2019-03-05 23:07:21 -0800, Andres Freund wrote: > > My next steps are: > > - final polish & push the basic DDL and pg_dump patches > > Done and pushed. Some collation dependent fallout, I'm hoping I've just > fixed that. > Thanks

Re: Pluggable Storage - Andres's take

2019-03-08 Thread Dagfinn Ilmari Mannsåker
Andres Freund writes: > On 2019-03-07 08:52:21 -0500, Robert Haas wrote: >> On Wed, Mar 6, 2019 at 6:11 PM Andres Freund wrote: >> > slot that's compatible with the "target" table. You can get compatible >> > slot callbakcs by calling table_slot_callbacks(), or directly create one >> > by

Re: Pluggable Storage - Andres's take

2019-03-07 Thread Robert Haas
On Thu, Mar 7, 2019 at 12:49 PM Andres Freund wrote: > On 2019-03-07 08:52:21 -0500, Robert Haas wrote: > > On Wed, Mar 6, 2019 at 6:11 PM Andres Freund wrote: > > > slot that's compatible with the "target" table. You can get compatible > > > slot callbakcs by calling table_slot_callbacks(), or

Re: Pluggable Storage - Andres's take

2019-03-07 Thread Andres Freund
On 2019-03-07 08:52:21 -0500, Robert Haas wrote: > On Wed, Mar 6, 2019 at 6:11 PM Andres Freund wrote: > > slot that's compatible with the "target" table. You can get compatible > > slot callbakcs by calling table_slot_callbacks(), or directly create one > > by calling table_gimmegimmeslot()

Re: Pluggable Storage - Andres's take

2019-03-07 Thread Robert Haas
On Wed, Mar 6, 2019 at 6:11 PM Andres Freund wrote: > slot that's compatible with the "target" table. You can get compatible > slot callbakcs by calling table_slot_callbacks(), or directly create one > by calling table_gimmegimmeslot() (likely to be renamed :)). Hmm. I assume the issue is that

Re: Pluggable Storage - Andres's take

2019-03-06 Thread Andres Freund
Hi, On 2019-03-07 11:56:57 +1300, David Rowley wrote: > On Thu, 7 Mar 2019 at 08:33, Andres Freund wrote: > > Here's a cleaned up version of that patch. David, Alvaro, you also > > played in that area, any objections? I think this makes that part of the > > code easier to read actually. Robert,

Re: Pluggable Storage - Andres's take

2019-03-06 Thread David Rowley
On Thu, 7 Mar 2019 at 08:33, Andres Freund wrote: > Here's a cleaned up version of that patch. David, Alvaro, you also > played in that area, any objections? I think this makes that part of the > code easier to read actually. Robert, thanks for looking at that patch > already. I only had a

Re: Pluggable Storage - Andres's take

2019-03-06 Thread Andres Freund
Hi, On 2019-03-05 23:07:21 -0800, Andres Freund wrote: > My next steps are: > - final polish & push the basic DDL and pg_dump patches Done and pushed. Some collation dependent fallout, I'm hoping I've just fixed that. > - cleanup & polish the ON CONFLICT refactoring Here's a cleaned up version

Re: Pluggable Storage - Andres's take

2019-03-05 Thread Andres Freund
Hi, Thanks for looking! On 2019-03-05 18:27:45 -0800, Ashwin Agrawal wrote: > While playing with the tableam, usage of which starts with commit > v12-0023-tableam-Introduce-and-use-begin-endscan-and-do-i.patch, should we > check for NULL function pointer before actually calling the same and

Re: Pluggable Storage - Andres's take

2019-03-05 Thread Andres Freund
Hi, Thanks for looking! On 2019-03-05 18:27:45 -0800, Ashwin Agrawal wrote: > While playing with the tableam, usage of which starts with commit > v12-0023-tableam-Introduce-and-use-begin-endscan-and-do-i.patch, should we > check for NULL function pointer before actually calling the same and

Re: Pluggable Storage - Andres's take

2019-03-05 Thread Ashwin Agrawal
Hi, While playing with the tableam, usage of which starts with commit v12-0023-tableam-Introduce-and-use-begin-endscan-and-do-i.patch, should we check for NULL function pointer before actually calling the same and ERROR out instead as NOT_SUPPORTED or something on those lines. Understand its

Re: Pluggable Storage - Andres's take

2019-02-27 Thread Andres Freund
Hi, On 2019-02-27 18:00:12 +0800, Heikki Linnakangas wrote: > I haven't been following this thread closely, but I looked briefly at some > of the patches posted here: Thanks! > On 21/01/2019 11:01, Andres Freund wrote: > > The patchset is now pretty granularly split into individual pieces. >

Re: Pluggable Storage - Andres's take

2019-02-27 Thread Heikki Linnakangas
I haven't been following this thread closely, but I looked briefly at some of the patches posted here: On 21/01/2019 11:01, Andres Freund wrote: The patchset is now pretty granularly split into individual pieces. Wow, 42 patches, very granular indeed! That's nice for reviewing, but are you

Re: Pluggable Storage - Andres's take

2019-02-26 Thread Haribabu Kommi
On Wed, Feb 27, 2019 at 11:10 AM Andres Freund wrote: > Hi, > > On 2019-01-21 10:32:37 +1100, Haribabu Kommi wrote: > > I am not able to remove the complete t_tableOid from HeapTuple, > > because of its use in triggers, as the slot is not available in triggers > > and I need to store the

Re: Pluggable Storage - Andres's take

2019-02-26 Thread Andres Freund
Hi, On 2019-01-21 10:32:37 +1100, Haribabu Kommi wrote: > I am not able to remove the complete t_tableOid from HeapTuple, > because of its use in triggers, as the slot is not available in triggers > and I need to store the tableOid also as part of the tuple. What precisely do you man by "use in

Re: Pluggable Storage - Andres's take

2019-02-25 Thread Amit Khandekar
On Sat, 23 Feb 2019 at 01:22, Robert Haas wrote: > > On Fri, Feb 22, 2019 at 11:19 AM Amit Khandekar > wrote: > > Thanks for the review. Attached v2. > > Thanks. I took this, combined it with Andres's > v12-0040-WIP-Move-xid-horizon-computation-for-page-level-.patch, did > some polishing of

Re: Pluggable Storage - Andres's take

2019-02-22 Thread Robert Haas
On Fri, Feb 22, 2019 at 11:19 AM Amit Khandekar wrote: > Thanks for the review. Attached v2. Thanks. I took this, combined it with Andres's v12-0040-WIP-Move-xid-horizon-computation-for-page-level-.patch, did some polishing of the code and comments, and pgindented. Here's what I ended up with;

Re: Pluggable Storage - Andres's take

2019-02-22 Thread Amit Khandekar
On Thu, 21 Feb 2019 at 18:06, Robert Haas wrote: > > On Thu, Feb 21, 2019 at 6:44 AM Amit Khandekar wrote: > > Ok, so something like XidHorizonPrefetchState ? On similar lines, does > > prefetch_buffer() function name sound too generic as well ? > > Yeah, that sounds good. > And, yeah, then

Re: Pluggable Storage - Andres's take

2019-02-21 Thread Robert Haas
On Thu, Feb 21, 2019 at 6:44 AM Amit Khandekar wrote: > Ok, so something like XidHorizonPrefetchState ? On similar lines, does > prefetch_buffer() function name sound too generic as well ? Yeah, that sounds good. And, yeah, then maybe rename the function too. > > +/* > > + * An arbitrary way

Re: Pluggable Storage - Andres's take

2019-02-21 Thread Amit Khandekar
On Thu, 21 Feb 2019 at 04:17, Robert Haas wrote: > > On Fri, Feb 8, 2019 at 5:18 AM Amit Khandekar wrote: > > In the attached v1 patch, the prefetch_distance is calculated as > > effective_io_concurrency + 10. Also it has some cosmetic changes. > > I did a little brief review of this patch and

Re: Pluggable Storage - Andres's take

2019-02-20 Thread Robert Haas
On Fri, Feb 8, 2019 at 5:18 AM Amit Khandekar wrote: > In the attached v1 patch, the prefetch_distance is calculated as > effective_io_concurrency + 10. Also it has some cosmetic changes. I did a little brief review of this patch and noticed the following things. +} PrefetchState; That name

Re: Pluggable Storage - Andres's take

2019-02-19 Thread Haribabu Kommi
On Tue, Nov 27, 2018 at 4:59 PM Amit Langote wrote: > Hi, > > On 2018/11/02 9:17, Haribabu Kommi wrote: > > Here I attached the cumulative fixes of the patches, new API additions > for > > zheap and > > basic outline of the documentation. > > I've read the documentation patch while also looking

Re: Pluggable Storage - Andres's take

2019-02-19 Thread Haribabu Kommi
On Mon, Feb 4, 2019 at 2:31 PM Haribabu Kommi wrote: > > On Tue, Jan 22, 2019 at 1:43 PM Haribabu Kommi > wrote: > >> >> >> OK. I will work on the doc changes. >> > > Sorry for the delay. > > Attached a draft patch of doc and comments changes that I worked upon. > Currently I added comments to

Re: Pluggable Storage - Andres's take

2019-02-08 Thread Amit Khandekar
On Wed, 6 Feb 2019 at 18:30, Amit Khandekar wrote: > > On Mon, 21 Jan 2019 at 08:31, Andres Freund wrote: > > > > Hi, > > > > (resending with compressed attachements, perhaps that'll go through) > > > > On 2018-12-10 18:13:40 -0800, Andres Freund wrote: > > > On 2018-11-26 17:55:57 -0800, Andres

Re: Pluggable Storage - Andres's take

2019-02-06 Thread Amit Khandekar
On Mon, 21 Jan 2019 at 08:31, Andres Freund wrote: > > Hi, > > (resending with compressed attachements, perhaps that'll go through) > > On 2018-12-10 18:13:40 -0800, Andres Freund wrote: > > On 2018-11-26 17:55:57 -0800, Andres Freund wrote: > > > FWIW, now that oids are removed, and the tuple

Re: Pluggable Storage - Andres's take

2019-02-03 Thread Haribabu Kommi
On Tue, Jan 22, 2019 at 1:43 PM Haribabu Kommi wrote: > > > OK. I will work on the doc changes. > Sorry for the delay. Attached a draft patch of doc and comments changes that I worked upon. Currently I added comments to the callbacks that are present in the TableAmRoutine structure and I

Re: Pluggable Storage - Andres's take

2019-01-31 Thread Amit Khandekar
Hi, Attached is a patch that adds some test scenarios for testing the dependency of various object types on table am. Besides simple tables, it considers materialized views, partitioned table, foreign table, and composite types, and verifies that the dependency is created only for those object

Re: Pluggable Storage - Andres's take

2019-01-28 Thread Dmitry Dolgov
> On Sun, Jan 20, 2019 at 6:17 PM Dmitry Dolgov <9erthali...@gmail.com> wrote: > > > On Fri, Jan 18, 2019 at 11:22 AM Amit Khandekar > > wrote: > > > > I believe you are going to add a new regression testcase for the change ? > > Yep. So, here are these two patches for pg_dump/psql with a few

Re: Pluggable Storage - Andres's take

2019-01-22 Thread Amit Khandekar
On Tue, 22 Jan 2019 at 15:29, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > On Mon, Jan 21, 2019 at 9:33 AM Amit Khandekar > > wrote: > > > > Regression tests that use \d+ to show the table details might > > not be interested specifically in table access method. But these will > > fail if run

Re: Pluggable Storage - Andres's take

2019-01-22 Thread Dmitry Dolgov
> On Mon, Jan 21, 2019 at 3:01 AM Andres Freund wrote: > > The patchset is now pretty granularly split into individual pieces. Wow, thanks! > On Mon, Jan 21, 2019 at 9:33 AM Amit Khandekar wrote: > > Regression tests that use \d+ to show the table details might > not be interested specifically

Re: Pluggable Storage - Andres's take

2019-01-21 Thread Haribabu Kommi
On Tue, Jan 22, 2019 at 12:15 PM Andres Freund wrote: > Hi, > > Thanks! > > On 2019-01-22 11:51:57 +1100, Haribabu Kommi wrote: > > Attached the patch for removal of scan_update_snapshot > > and also the rebased patch of reduction in use of t_tableOid. > > I'll soon look at the latter. >

Re: Pluggable Storage - Andres's take

2019-01-21 Thread Andres Freund
Hi, Thanks! On 2019-01-22 11:51:57 +1100, Haribabu Kommi wrote: > Attached the patch for removal of scan_update_snapshot > and also the rebased patch of reduction in use of t_tableOid. I'll soon look at the latter. > > - consider removing table_gimmegimmeslot() > > - add substantial docs for

Re: Pluggable Storage - Andres's take

2019-01-21 Thread Haribabu Kommi
On Mon, Jan 21, 2019 at 1:01 PM Andres Freund wrote: > Hi, > > On 2018-12-10 18:13:40 -0800, Andres Freund wrote: > > On 2018-11-26 17:55:57 -0800, Andres Freund wrote: > > > FWIW, now that oids are removed, and the tuple table slot abstraction > > > got in, I'm working on rebasing the pluggable

Re: Pluggable Storage - Andres's take

2019-01-21 Thread Amit Khandekar
On Sun, 20 Jan 2019 at 22:46, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > > On Fri, Jan 18, 2019 at 11:22 AM Amit Khandekar > > wrote: > > > > --- a/src/test/regress/expected/copy2.out > > +++ b/src/test/regress/expected/copy2.out > > @@ -1,3 +1,4 @@ > > +\set HIDE_TABLEAM on > > > > I

Re: Pluggable Storage - Andres's take

2019-01-20 Thread Haribabu Kommi
On Tue, Jan 15, 2019 at 6:05 PM Andres Freund wrote: > Hi, > > On 2019-01-15 18:02:38 +1100, Haribabu Kommi wrote: > > On Tue, Dec 11, 2018 at 1:13 PM Andres Freund > wrote: > > > > > Hi, > > > > > > On 2018-11-26 17:55:57 -0800, Andres Freund wrote: > > > Further tasks I'm not yet planning to

Re: Pluggable Storage - Andres's take

2019-01-20 Thread Dmitry Dolgov
> On Fri, Jan 18, 2019 at 11:22 AM Amit Khandekar > wrote: > > --- a/src/test/regress/expected/copy2.out > +++ b/src/test/regress/expected/copy2.out > @@ -1,3 +1,4 @@ > +\set HIDE_TABLEAM on > > I thought we wanted to avoid having to add this setting in individual > regression tests. Can't we do

Re: Pluggable Storage - Andres's take

2019-01-18 Thread Amit Khandekar
On Fri, 18 Jan 2019 at 10:13, Amit Khandekar wrote: > On Tue, 15 Jan 2019 at 17:58, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > Also I guess another attached patch should address the psql part, namely > > displaying a table access method with \d+ and possibility to hide it with a > > psql

Re: Pluggable Storage - Andres's take

2019-01-17 Thread Amit Khandekar
On Tue, 15 Jan 2019 at 17:58, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > > On Tue, Jan 15, 2019 at 10:52 AM Amit Khandekar > > wrote: > > > > Need to bump K_VERS_MINOR as well. > > I've bumped it up, but somehow this change escaped the previous version. Now > should be there, thanks! > > >

Re: Pluggable Storage - Andres's take

2019-01-15 Thread Andres Freund
Hi, On 2019-01-15 14:37:36 +0530, Amit Khandekar wrote: > Then for each of the calls, we would need to declare that structure > variable (with = {0}) and assign required fields in that structure > before passing it to ArchiveEntry(). But a major reason of > ArchiveEntry() is to avoid doing this

Re: Pluggable Storage - Andres's take

2019-01-15 Thread Dmitry Dolgov
> On Tue, Jan 15, 2019 at 10:52 AM Amit Khandekar > wrote: > > Need to bump K_VERS_MINOR as well. I've bumped it up, but somehow this change escaped the previous version. Now should be there, thanks! > On Mon, 14 Jan 2019 at 18:36, Amit Khandekar wrote: > > +static void

Re: Pluggable Storage - Andres's take

2019-01-15 Thread Amit Khandekar
On Tue, 15 Jan 2019 at 12:27, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > > On Mon, Jan 14, 2019 at 2:07 PM Amit Khandekar > > wrote: > > > > createPQExpBuffer() should be moved after the below statement, so that > > it does not leak memory > > Thanks for noticing, fixed. Looks good. > >

Re: Pluggable Storage - Andres's take

2019-01-15 Thread Amit Khandekar
On Sat, 12 Jan 2019 at 18:11, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > On Sat, Jan 12, 2019 at 1:44 AM Andres Freund wrote: > > > /* other fields were zeroed above */ > > > > > > @@ -9355,7 +9370,7 @@ dumpComment(Archive *fout, const char *type, const > > > char *name, > >

Re: Pluggable Storage - Andres's take

2019-01-14 Thread Andres Freund
Hi, On 2019-01-15 18:02:38 +1100, Haribabu Kommi wrote: > On Tue, Dec 11, 2018 at 1:13 PM Andres Freund wrote: > > > Hi, > > > > On 2018-11-26 17:55:57 -0800, Andres Freund wrote: > > Further tasks I'm not yet planning to tackle, that I'd welcome help on: > > - pg_upgrade testing > > > > I did

  1   2   >