On Mon, Aug 11, 2025 at 05:39:01PM +0800, Japin Li wrote: > On Fri, Aug 08, 2025 at 06:08:27PM +1000, Peter Smith wrote: > > Here are the latest v17 patches. > > > > Changes include: > > > > PATCH 0002. > > - Rebase to fix compile error, result of recent master change > > - Bugfix for an unreported EXPLAIN ANALYZE error > > - Bugfix for an unreported double pfree > > - Code cleanup (ran pgindent; corrected ~100 typos; removed empty > > coverage comments) > > > 3. > Here are some typos. > > a) > @@ -475,7 +477,7 @@ vci_scan_EndCustomPlan(CustomScanState *node) > > default: > /* LCOV_EXCL_START */ > - elog(PANIC, "Should not reach hare"); > + elog(PANIC, "Should not reach here"); > /* LCOV_EXCL_STOP */ > break; > } > b) > @@ -543,7 +545,7 @@ vci_create_relation(const char *rel_identifier, Relation > indexRel, IndexInfo *in > TupleDescInitEntry(new_tupdesc, (AttrNumber) 1, "bindata", > BYTEAOID, -1, 0); /* */ > break; > > - /* TIC-CRID */ > + /* TID-CRID */ > case VCI_RELTYPE_TIDCRID: > natts = 1; > new_tupdesc = CreateTemplateTupleDesc(natts); /* no Oid */ > > c) > @@ -1065,7 +1065,7 @@ vci_inner_build(Relation heapRel, Relation indexRel, > IndexInfo *indexInfo) > /* > * Put or Copy page into INIT_FORK. > * If valid page is given, that page will be putted into INIT_FORK. > - * If Invalid page (NULL pointer) is given, MAIN_FORK page well be copied. > + * If invalid page (NULL pointer) is given, MAIN_FORK page well be copied. > */ > static void > vci_putInitPage(Oid oid, Page page, BlockNumber blkno) > > 1. PFA the other typos.
2. I found it skip vci query context initialization in vci_intialize_query_context() if full page writes is disabled, Could you explain why we need full page write enabled for VCI? 3. Both vci_ros.h and vci_ros.c have a comment about accessing the VCI main relation header, but they are slightly different. Could we sync them and keep only one? It seems the comment is outdated, as the functions vci_KeepReadingMainRelHeader() and vci_KeepWritingMainRelHeader() do not exist in the current VCI implementation. 4. +/** + * This function is assumed when the VCI index is newly built, and + * it converts all the data in the relation of PostgreSQL into ROS. + */ +uint64 +vci_ConvertWos2RosForBuild(Relation mainRel, + Size workareaSize, + IndexInfo *indexInfo) ... + result = (uint64) table_index_build_scan(comContext.heapRel, + mainRel, + indexInfo, + true, /* allow syncscan */ + true, + vci_build_callback, + (void *) &comContext, NULL) Perhaps we can use a double return type to avoid type casting since other places also use double. -- Best regards, Japin Li ChengDu WenWu Information Technology Co., LTD.
diff --git a/contrib/vci/include/vci_ros.h b/contrib/vci/include/vci_ros.h index b8f993911f7..db6b6515f4a 100644 --- a/contrib/vci/include/vci_ros.h +++ b/contrib/vci/include/vci_ros.h @@ -271,7 +271,7 @@ vci_GetNumBlocks(Size size) /* Accessing VCI main relation header - * Because the header ov VCI main relation has three pages, we can not map + * Because the header of VCI main relation has three pages, we can not map * one structure of C on the header pages simply. * Instead, we use access functions. * diff --git a/contrib/vci/storage/vci_ros.c b/contrib/vci/storage/vci_ros.c index 2063f70d717..a92e51a8b8f 100644 --- a/contrib/vci/storage/vci_ros.c +++ b/contrib/vci/storage/vci_ros.c @@ -471,11 +471,11 @@ WriteAllItemsInPage(Relation rel, * This function checks if the relation has the DB page pointed * by an argument. If it does not exists, the function extends * the relation and initialize extended pages with one item per - * page. Mind that this function do not touch existing pages. + * page. Mind that this function does not touch existing pages. * If you need to format existing pages, use vci_InitPage(). * * vci_InitPage() - * Low level function. The function, + * Low level function. * * This function formats the existing DB page, pointed by * relation and page ID (block number), with empty items. @@ -566,7 +566,7 @@ vci_PreparePagesIfNecessaryCore(Relation rel, /** * @brief - * This function write items of given number in the buffer. + * This function writes items of given number in the buffer. * @param[in] buffer Postgres DB buffer to be initialized. * @param[in] numItems The number of items the page is initialized with. * @param[in] locked true if the buffer is locked, false otherwise. @@ -1616,7 +1616,7 @@ ReadBufferWithPageInitCore(Relation reln, BlockNumber blockNumber, int16 numItem * don't need to replace ReadBuffer() immediately after vci_PreparePagesIfNecessaryCore(). * * @param[in] reln The relation. - * @param[in] blockNumber The block number to be red. + * @param[in] blockNumber The block number to be read. */ Buffer vci_ReadBufferWithPageInit(Relation reln, BlockNumber blockNumber) @@ -1635,7 +1635,7 @@ vci_ReadBufferWithPageInit(Relation reln, BlockNumber blockNumber) * vci_PreparePagesIfNecessaryCore(). * * @param[in] reln The relation. - * @param[in] blockNumber The block number to be red. + * @param[in] blockNumber The block number to be read. */ Buffer vci_ReadBufferWithPageInitDelVec(Relation reln, BlockNumber blockNumber) diff --git a/contrib/vci/storage/vci_wos.c b/contrib/vci/storage/vci_wos.c index af23a518637..bcb806cf54f 100644 --- a/contrib/vci/storage/vci_wos.c +++ b/contrib/vci/storage/vci_wos.c @@ -103,7 +103,7 @@ static TransactionId exclusiveTransactionId; * Creates a snapshot which is used for local ROS conversion * * @param[in] inclusive_xid Visible xid regardless of the MVCC snapshot - * @param[in] exclusive_xid Inisible xid regardless of the MVCC snapshot + * @param[in] exclusive_xid Invisible xid regardless of the MVCC snapshot * * Mostly same as vci_GetSnapshotForWos2Ros(), but sometimes results by ROS * control commands cannot be seen by MVCC. Because the transactions creating
