You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by "avamingli (via GitHub)" <gi...@apache.org> on 2025/03/12 08:13:00 UTC
[PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
avamingli opened a new pull request, #990:
URL: https://github.com/apache/cloudberry/pull/990
Previously, write operations on partitioned tables would alter the materialized view (MV) status of both ancestor and descendant tables, leading to unnecessary invalidations of MVs that depended on unaffected partitions.
This commit introduces a more efficient approach to updating MV statuses for partitioned tables by focusing on leaf partitions that have actually undergone data changes.
## example:
```sql
create table par(a int, b int, c int) partition by range(b)
subpartition by range(c) subpartition template (start (1) end (3) every (1))
(start(1) end(3) every(1));
insert into par values(1, 1, 1), (1, 1, 2), (2, 2, 1), (2, 2, 2);
create materialized view mv_par as select * from par;
create materialized view mv_par1 as select * from par_1_prt_1;
create materialized view mv_par1_1 as select * from par_1_prt_1_2_prt_1;
create materialized view mv_par1_2 as select * from par_1_prt_1_2_prt_2;
create materialized view mv_par2 as select * from par_1_prt_2;
create materialized view mv_par2_2 as select * from par_1_prt_2_2_prt2;
```
### INSERT INTO Partitioned Table
```sql
INSERT INTO P1 VALUES (1, 1, 1);
```
#### before this commit:
It is known that data was successfully inserted into P1, which is a descendant node of P0. Due to the increased data, the status needs to be propagated upwards to invalidate the materialized view MV_0 based on P0.
At the same time, P1 is a partitioned table, and it also needs to propagate the status downwards to its child tables. Since it is unclear which child tables have undergone data changes, all materialized views of the child tables must be invalidated.
Consequently, the materialized views MV_0, MV_1, MV_1_1, and MV_1_2 are all invalidated.
#### after this commit:
When data is inserted into a partitioned table, it is ultimately routed to the child tables at the leaf nodes. The system monitors these leaf nodes for any data changes and propagates the status upward, while ignoring the changes directly made to the partition table P0 in the SQL.
The tuple (1, 1, 1) will be routed to the partition P1_1, invalidating the materialized views of P1_1 and its ancestor partition tables. In contrast, the other sub-partition P1_2 experiences no data changes, so the materialized view MV_1_2 remains valid.
As a result, the materialized views MV_0, MV_1, and MV_1_1 are invalidated, while MV_1_2 remains unaffected.
The new approach allows for status propagation only from the leaf nodes upward, rather than sending status updates in both directions through the intermediate nodes of the partition tree. This safeguards unrelated materialized views from unnecessary refreshes.
### (Split) Across Partition Update on root table
```sql
UPDATE P0 SET c = 2 WHERE b = 2 AND c = 1;
```
#### before this commit:
When the data in the root node is updated, and since it is unclear which specific child table was updated, the status is propagated downwards to all descendant child tables.
As a result, all materialized views based on the partition tree are invalidated: MV_0, MV_1, MV_1_1, MV_1_2, and MV_2_2.
#### after this commit:
Since `c` is the partition key, updating the partition key will cause data movement **across partitions** in the child tables. The **UPDATE** operation on the parent table will be translated into an **INSERT** for one child table and a **DELETE** for another.
The system monitors the changed leaf node child tables: P2_1 and P2_2, and only propagates their respective statuses upward, invalidating the materialized views based on their ancestor tables. The materialized views related to the left subtree P1 remain unaffected.
As a result, MV_0 and MV_2_2 are invalidated, while MV_1, MV_1_1, and MV_1_2 remain valid.
## Extend protocol of libpq
The executor (QE) records the modified partition child tables during the data modification process. A mechanism is needed to transmit these results to the QD for updating the metadata. The libpq protocol has been extended to support this data transfer.
### Processing on the QE and QD sides:
Each QE records the IDs of the child tables it has modified, deduplicates them in a local bitmap, and sends this information to the QD.
The data changes on each segment node may not be the same. For example, in the diagram, seg0 has changes in two tables, while seg1 has no changes (as the data to be updated is not located on the seg1 node due to distribution).
The QD collects multiple bitmaps returned from the segment nodes and consolidates them into a single bitmap, removing duplicate relids. This deduplicated bitmap is then used to update the status of the ancestor nodes.
### Extend Protocol Design
Based on libpq, the protocol has been extended with an **'e'** protocol, which stands for "extend protocol," to unify the handling of information returns.
Each transmission begins with the **'e'** protocol, followed immediately by data blocks. A single transmission can carry multiple data blocks.
Each data block corresponds to a specific type of data and is distinguished by a **subtag**. Following the subtag is a 32-bit integer, **num**, which indicates the total number of relids in the subsequent array.
The relid array follows the num value. After all data blocks have been written, the transmission concludes with a **subtag_max**.
The **subtag_max**, which belongs to the same enum category as the **subtag**, is specifically used to indicate the termination of an extended protocol transmission.
## Key changes:
- Leaf Partition Tracking: Instead of updating the MV status based on the parent table, we now detect and track changes at the leaf partition level. This ensures that only partitions with actual data modifications trigger MV status updates, significantly reducing unnecessary refreshes.
- Cross-Partition Updates: For operations like UPDATEs that span multiple partitions (e.g., decomposed into an INSERT on one leaf and a DELETE on another), the MV status is updated based on the real data state of the affected leaf partitions, rather than propagating changes through the parent table. This avoids invalidating MVs that depend on unrelated partitions.
- Executor Enhancements: The executor now detects dynamic partition expansion during query execution and records data modification bitmaps for each partition on the QE. These bitmaps are aggregated on the QD to update MV statuses accurately.
- Protocol Extension: The libpq protocol has been extended to handle QE feedback uniformly, ensuring that partition-level modification information is properly collected and consumed by the QD.
This optimization minimizes the impact on MVs by ensuring that only relevant partitions trigger status updates, improving performance and reducing unnecessary invalidations.
Authored-by: Zhang Mingli avamingli@gmail.com
<!-- Thank you for your contribution to Apache Cloudberry (Incubating)! -->
Fixes #ISSUE_Number
### What does this PR do?
<!-- Brief overview of the changes, including any major features or fixes -->
### Type of Change
- [ ] Bug fix (non-breaking change)
- [ ] New feature (non-breaking change)
- [ ] Breaking change (fix or feature with breaking changes)
- [ ] Documentation update
### Breaking Changes
<!-- Remove if not applicable. If yes, explain impact and migration path -->
### Test Plan
<!-- How did you test these changes? -->
- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] Passed `make installcheck`
- [ ] Passed `make -C src/test installcheck-cbdb-parallel`
### Impact
<!-- Remove sections that don't apply -->
**Performance:**
<!-- Any performance implications? -->
**User-facing changes:**
<!-- Any changes visible to users? -->
**Dependencies:**
<!-- New dependencies or version changes? -->
### Checklist
- [ ] Followed [contribution guide](https://cloudberry.apache.org/contribute/code)
- [ ] Added/updated documentation
- [ ] Reviewed code for security implications
- [ ] Requested review from [cloudberry committers](https://github.com/orgs/apache/teams/cloudberry-committers)
### Additional Context
<!-- Any other information that would help reviewers? Remove if none -->
### CI Skip Instructions
<!--
To skip CI builds, add the appropriate CI skip identifier to your PR title.
The identifier must:
- Be in square brackets []
- Include the word "ci" and either "skip" or "no"
- Only use for documentation-only changes or when absolutely necessary
-->
---
<!-- Join our community:
- Mailing list: [dev@cloudberry.apache.org](https://lists.apache.org/list.html?dev@cloudberry.apache.org) (subscribe: dev-subscribe@cloudberry.apache.org)
- Discussions: https://github.com/apache/cloudberry/discussions -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli merged PR #990:
URL: https://github.com/apache/cloudberry/pull/990
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "yjhjstz (via GitHub)" <gi...@apache.org>.
yjhjstz commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2001607856
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
+ bms_free(list_nth(epd->subtagdata, subtag)); /* clean up */
+ break;
+ default:
+ Assert(false);
+ }
+
+ /* Mark subtag consumed. */
+ epd->consumed_bitmap &= ~(1 << subtag);
+}
+
+/*
+ * Contents must be allocated in TopTransactionMemoryContext.
+ */
+void InitExtendProtocolData(void)
+{
+ MemoryContext oldctx = MemoryContextSwitchTo(TopTransactionContext);
Review Comment:
ExtendProtocol data struct could be more general ？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "yjhjstz (via GitHub)" <gi...@apache.org>.
yjhjstz commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r1996424384
##########
src/backend/executor/nodeModifyTable.c:
##########
@@ -3654,6 +3709,70 @@ ExecEndModifyTable(ModifyTableState *node)
* shut down subplan
*/
ExecEndNode(outerPlanState(node));
+
+ /* Notify modified leaf relids to QD */
+ if (GP_ROLE_EXECUTE == Gp_role && node->has_leaf_changed)
+ notify_modified_relations_to_QD(node);
+}
+
+/*
+ * notify_modified_relations_local
+ * For SINGLENODE or we are entry db, update the modified relids on local.
+ * To keep consistent, we set the extend protocol data which will be processed
+ * uniformly later at the end of exetuor run.
+ */
+static void
+notify_modified_relations_local(ModifyTableState *node)
+{
+ Assert(epd);
Review Comment:
when reset epd ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "yjhjstz (via GitHub)" <gi...@apache.org>.
yjhjstz commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r1996422467
##########
src/interfaces/libpq/fe-protocol3.c:
##########
@@ -475,6 +476,17 @@ pqParseInput3(PGconn *conn)
*/
break;
#ifndef FRONTEND
Review Comment:
if defined FRONTEND, will any warnings ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2032298893
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
+ bms_free(list_nth(epd->subtagdata, subtag)); /* clean up */
+ break;
+ default:
+ Assert(false);
+ }
+
+ /* Mark subtag consumed. */
+ epd->consumed_bitmap &= ~(1 << subtag);
+}
+
+/*
+ * Contents must be allocated in TopTransactionMemoryContext.
+ */
+void InitExtendProtocolData(void)
+{
+ MemoryContext oldctx = MemoryContextSwitchTo(TopTransactionContext);
Review Comment:
Eh...embed json/jsonb inside kernel? Surprised to hear that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "yjhjstz (via GitHub)" <gi...@apache.org>.
yjhjstz commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2032312550
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
+ bms_free(list_nth(epd->subtagdata, subtag)); /* clean up */
+ break;
+ default:
+ Assert(false);
+ }
+
+ /* Mark subtag consumed. */
+ epd->consumed_bitmap &= ~(1 << subtag);
+}
+
+/*
+ * Contents must be allocated in TopTransactionMemoryContext.
+ */
+void InitExtendProtocolData(void)
+{
+ MemoryContext oldctx = MemoryContextSwitchTo(TopTransactionContext);
Review Comment:
> Eh...embed json/jsonb inside kernel? Surprised to hear that.
https://www.postgresql.org/docs/9.5/functions-json.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on PR #990:
URL: https://github.com/apache/cloudberry/pull/990#issuecomment-2720806315
Added commit: e5c13e7a4b9bd0e519da9012a6e1cc1a92f45fa9
SINGLENODE and EntryDB materialized view status maintenance optimization.
In SINGLENODE mode or when operating as an entry db, we cannot use
the extend libpq to send messages because we are already functioning in
a QD role. Processing modified relations at `EndModifyTable()` is
too late, especially since materialized views are updated when
the executor ends.
This change moves the processing of modified relations to
`ExecModifyTable()` to ensure timely handling of modifications. This
avoids issues with materialized view updates and ensures correct
behavior in SINGLENODE or entry DB scenarios.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "gfphoenix78 (via GitHub)" <gi...@apache.org>.
gfphoenix78 commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2034234716
##########
src/backend/executor/nodeModifyTable.c:
##########
@@ -3654,6 +3709,70 @@ ExecEndModifyTable(ModifyTableState *node)
* shut down subplan
*/
ExecEndNode(outerPlanState(node));
+
+ /* Notify modified leaf relids to QD */
+ if (GP_ROLE_EXECUTE == Gp_role && node->has_leaf_changed)
+ notify_modified_relations_to_QD(node);
Review Comment:
What's the behavior of modified status propagation in single-node or utility mode?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "yjhjstz (via GitHub)" <gi...@apache.org>.
yjhjstz commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2031667874
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
+ bms_free(list_nth(epd->subtagdata, subtag)); /* clean up */
+ break;
+ default:
+ Assert(false);
+ }
+
+ /* Mark subtag consumed. */
+ epd->consumed_bitmap &= ~(1 << subtag);
+}
+
+/*
+ * Contents must be allocated in TopTransactionMemoryContext.
+ */
+void InitExtendProtocolData(void)
+{
+ MemoryContext oldctx = MemoryContextSwitchTo(TopTransactionContext);
Review Comment:
I suggest use json or jsonb to more general.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2002989311
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
+ bms_free(list_nth(epd->subtagdata, subtag)); /* clean up */
+ break;
+ default:
+ Assert(false);
+ }
+
+ /* Mark subtag consumed. */
+ epd->consumed_bitmap &= ~(1 << subtag);
+}
+
+/*
+ * Contents must be allocated in TopTransactionMemoryContext.
+ */
+void InitExtendProtocolData(void)
+{
+ MemoryContext oldctx = MemoryContextSwitchTo(TopTransactionContext);
Review Comment:
The `ExtendProtocol` data struct is designed to allow users to define their own extended subtag data list, so it’s flexible based on their specific needs.
That said, agree that the struct could be more general, and we do have plans to refactor it, along with `libpq`, to improve its versatility. However, these changes won’t be included in this PR and will be addressed in a future update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "gfphoenix78 (via GitHub)" <gi...@apache.org>.
gfphoenix78 commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2034234716
##########
src/backend/executor/nodeModifyTable.c:
##########
@@ -3654,6 +3709,70 @@ ExecEndModifyTable(ModifyTableState *node)
* shut down subplan
*/
ExecEndNode(outerPlanState(node));
+
+ /* Notify modified leaf relids to QD */
+ if (GP_ROLE_EXECUTE == Gp_role && node->has_leaf_changed)
+ notify_modified_relations_to_QD(node);
Review Comment:
What's the behavior of modified status propagation in single-node or utility mode?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2032368256
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
+ bms_free(list_nth(epd->subtagdata, subtag)); /* clean up */
+ break;
+ default:
+ Assert(false);
+ }
+
+ /* Mark subtag consumed. */
+ epd->consumed_bitmap &= ~(1 << subtag);
+}
+
+/*
+ * Contents must be allocated in TopTransactionMemoryContext.
+ */
+void InitExtendProtocolData(void)
+{
+ MemoryContext oldctx = MemoryContextSwitchTo(TopTransactionContext);
Review Comment:
No such need, it makes no sense to make things complicated.
And json/jsonb is a database object, it's very stupid to make them embedding inside internal codes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on PR #990:
URL: https://github.com/apache/cloudberry/pull/990#issuecomment-2785241562
TPC-B
**before this commit:**
| round | avg | round 1 | round 2 | round 3 |
|-------------|--------------|-------------|-------------|-------------|
| transactions| 367850 | 368535 | 368258 | 366757 |
| latency | 16.30733333 | 16.277 | 16.289 | 16.356 |
| TPS | 1226.43066 | 1228.694187 | 1227.83674 | 1222.761054 |
**after this commit:**
| round | avg | round 1 | round 2 | round 3 |
|--------------|--------------|-------------|-------------|-------------|
| transactions | 370138 | 368501 | 370823 | 371090 |
| latency(ms) | 16.20633333 | 16.279 | 16.176 | 16.164 |
| TPS | 1234.093081 | 1228.564701 | 1236.364521 | 1237.350022 |
**Summary**
| Summary | with PR 990 |
|--------------|----------|
| transactions | +0.62% |
| latency | -0.62% |
| TPS | +0.62% |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "yjhjstz (via GitHub)" <gi...@apache.org>.
yjhjstz commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r1997863861
##########
src/backend/executor/nodeModifyTable.c:
##########
@@ -3654,6 +3709,70 @@ ExecEndModifyTable(ModifyTableState *node)
* shut down subplan
*/
ExecEndNode(outerPlanState(node));
+
+ /* Notify modified leaf relids to QD */
+ if (GP_ROLE_EXECUTE == Gp_role && node->has_leaf_changed)
+ notify_modified_relations_to_QD(node);
+}
+
+/*
+ * notify_modified_relations_local
+ * For SINGLENODE or we are entry db, update the modified relids on local.
+ * To keep consistent, we set the extend protocol data which will be processed
+ * uniformly later at the end of exetuor run.
+ */
+static void
+notify_modified_relations_local(ModifyTableState *node)
+{
+ Assert(epd);
Review Comment:
I find list_make1 new list , but no free old list ? Am I right ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "yjhjstz (via GitHub)" <gi...@apache.org>.
yjhjstz commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r1996380053
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
Review Comment:
here Bitmapset need to consider little or big endian .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r1997965864
##########
src/backend/executor/nodeModifyTable.c:
##########
@@ -3654,6 +3709,70 @@ ExecEndModifyTable(ModifyTableState *node)
* shut down subplan
*/
ExecEndNode(outerPlanState(node));
+
+ /* Notify modified leaf relids to QD */
+ if (GP_ROLE_EXECUTE == Gp_role && node->has_leaf_changed)
+ notify_modified_relations_to_QD(node);
+}
+
+/*
+ * notify_modified_relations_local
+ * For SINGLENODE or we are entry db, update the modified relids on local.
+ * To keep consistent, we set the extend protocol data which will be processed
+ * uniformly later at the end of exetuor run.
+ */
+static void
+notify_modified_relations_local(ModifyTableState *node)
+{
+ Assert(epd);
Review Comment:
When consume subtag list, it's already been cleaned up
```c
bms_free(list_nth(epd->subtagdata, subtag)); /* clean up */
```
And the list pointer which is tiny will be freed with MemoryContext.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r1996634550
##########
src/backend/executor/nodeModifyTable.c:
##########
@@ -3654,6 +3709,70 @@ ExecEndModifyTable(ModifyTableState *node)
* shut down subplan
*/
ExecEndNode(outerPlanState(node));
+
+ /* Notify modified leaf relids to QD */
+ if (GP_ROLE_EXECUTE == Gp_role && node->has_leaf_changed)
+ notify_modified_relations_to_QD(node);
+}
+
+/*
+ * notify_modified_relations_local
+ * For SINGLENODE or we are entry db, update the modified relids on local.
+ * To keep consistent, we set the extend protocol data which will be processed
+ * uniformly later at the end of exetuor run.
+ */
+static void
+notify_modified_relations_local(ModifyTableState *node)
+{
+ Assert(epd);
Review Comment:
see callers of InitExtendProtocolData().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r1996634074
##########
src/interfaces/libpq/fe-protocol3.c:
##########
@@ -475,6 +476,17 @@ pqParseInput3(PGconn *conn)
*/
break;
#ifndef FRONTEND
Review Comment:
No, it's quite common for this style.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r1996493400
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
Review Comment:
Why that matters?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "yjhjstz (via GitHub)" <gi...@apache.org>.
yjhjstz commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r1996421749
##########
src/interfaces/libpq/fe-protocol3.c:
##########
@@ -475,6 +476,17 @@ pqParseInput3(PGconn *conn)
*/
break;
#ifndef FRONTEND
+ case 'e':
Review Comment:
add comments like upper code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2032317009
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
+ bms_free(list_nth(epd->subtagdata, subtag)); /* clean up */
+ break;
+ default:
+ Assert(false);
+ }
+
+ /* Mark subtag consumed. */
+ epd->consumed_bitmap &= ~(1 << subtag);
+}
+
+/*
+ * Contents must be allocated in TopTransactionMemoryContext.
+ */
+void InitExtendProtocolData(void)
+{
+ MemoryContext oldctx = MemoryContextSwitchTo(TopTransactionContext);
Review Comment:
> > Eh...embed json/jsonb inside kernel? Surprised to hear that.
>
> https://www.postgresql.org/docs/9.5/functions-json.html
So what and why?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Optimize Materialized View Status Maintenance for Partitioned Tables [cloudberry]
Posted by "yjhjstz (via GitHub)" <gi...@apache.org>.
yjhjstz commented on code in PR #990:
URL: https://github.com/apache/cloudberry/pull/990#discussion_r2032329147
##########
src/backend/cdb/dispatcher/cdbdisp_query.c:
##########
@@ -1676,3 +1681,104 @@ findParamType(List *params, int paramid)
return InvalidOid;
}
+
+/*
+ * process data in pointer cosume_p, ex: copy everything interested in.
+ * For EP_TAG_I, EP_TAG_U, EP_TAG_D, consume_p is a Bitmapset**, we just
+ * copy the content and process them later.
+ */
+void
+ConsumeExtendProtocolData(ExtendProtocolSubTag subtag, void *consume_p)
+{
+ Assert(epd);
+
+ if ((epd->consumed_bitmap & (1 << subtag)) == 0)
+ return;
+
+ switch (subtag)
+ {
+ case EP_TAG_I:
+ case EP_TAG_U:
+ case EP_TAG_D:
+ Assert(consume_p != NULL);
+ *((Bitmapset **) consume_p) = bms_copy(list_nth(epd->subtagdata, subtag));
+ bms_free(list_nth(epd->subtagdata, subtag)); /* clean up */
+ break;
+ default:
+ Assert(false);
+ }
+
+ /* Mark subtag consumed. */
+ epd->consumed_bitmap &= ~(1 << subtag);
+}
+
+/*
+ * Contents must be allocated in TopTransactionMemoryContext.
+ */
+void InitExtendProtocolData(void)
+{
+ MemoryContext oldctx = MemoryContextSwitchTo(TopTransactionContext);
Review Comment:
to make extend protocol of libpq schema less.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org