agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH v9 4/4] WIP: check tuple size 26+ messages / 3 participants [nested] [flat]
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v8 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 47a5271b91..b4f523e32d 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --AZuoSAvZwvV/ife4-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v9 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 2059428e2a..04b0598183 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -703,7 +703,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --3yNHWXBV/QO9xKNm-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v10 4/4] WIP: check tuple size @ 2020-12-01 23:20 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Justin Pryzby @ 2020-12-01 23:20 UTC (permalink / raw) Or maybe INSERT should flush buffer based only on the *number* of tuples, and not their size ? --- src/backend/executor/nodeModifyTable.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index e018946c0e..ec9e43ac55 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -760,7 +760,13 @@ ExecInsert(ModifyTableState *mtstate, batchslot = MultiInsertInfoNextFreeSlot(mtstate->miinfo, resultRelInfo); ExecCopySlot(batchslot, slot); - MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, 0, 0); // XXX: tuplen/lineno + MultiInsertInfoStore(mtstate->miinfo, resultRelInfo, batchslot, + // sizeof(void*) * batchslot->tts_nvalid, /* tuple size - underestimate */ + MemoryContextMemAllocated(batchslot->tts_mcxt, true), /* tuple size */ + mtstate->ntuples); /* lineno */ + + elog(DEBUG2, "bufferedBytes %d; tuples %ld", + mtstate->miinfo->bufferedBytes, mtstate->ntuples); if (MultiInsertInfoIsFull(mtstate->miinfo)) MultiInsertInfoFlush(mtstate->miinfo, resultRelInfo); -- 2.17.0 --8nsIa27JVQLqB7/C-- ^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: An inefficient query caused by unnecessary PlaceHolderVar @ 2024-12-02 03:46 Richard Guo <[email protected]> 0 siblings, 1 reply; 26+ messages in thread From: Richard Guo @ 2024-12-02 03:46 UTC (permalink / raw) To: Dmitry Dolgov <[email protected]>; +Cc: James Coleman <[email protected]>; pgsql-hackers On Wed, Nov 27, 2024 at 5:45 PM Richard Guo <[email protected]> wrote: > I ended up using 'under the same lowest nulling outer join' to > keep consistent with the wording used elsewhere. Please see the > updated patch attached. Commit e032e4c7d computes the nullingrel data for each leaf RTE, and we can leverage that to determine if the referenced rel is under the same lowest nulling outer join: we just need to check if the nullingrels of the subquery RTE are a subset of those of the lateral referenced rel. This eliminates the need to introduce lowest_nullable_side. Please see attached. > BTW, since commit cb8e50a4a, we've chosen not to wrap a non-var > expression if it contains Vars/PHVs of the pulled-up subquery and does > not contain non-strict constructs. I wonder if we can apply the same > optimization from this patch to non-var expressions: for a LATERAL > subquery, if a non-var expression contains Vars/PHVs of the > lowest_nullable_relids and does not contain non-strict constructs, it > could also escape being wrapped. Any thoughts? I still feel that we can apply a similar optimization to more complex non-var expressions. I plan to explore that in a separate patch. Thanks Richard Attachments: [application/octet-stream] v6-0001-Avoid-unnecessary-wrapping-for-Vars-and-PHVs.patch (11.2K, ../../CAMbWs49T36=VrF0mBjiQXRmMCTOGzEsRgFULsiKX6TQMXXHX1g@mail.gmail.com/2-v6-0001-Avoid-unnecessary-wrapping-for-Vars-and-PHVs.patch) download | inline diff: From cd9f79c71e0bdcc8cbf62f319649d5359a74ea7b Mon Sep 17 00:00:00 2001 From: Richard Guo <[email protected]> Date: Mon, 2 Dec 2024 10:31:39 +0900 Subject: [PATCH v6] Avoid unnecessary wrapping for Vars and PHVs When pulling up a lateral subquery that is under an outer join, the current code always wraps a Var or PHV in the subquery's targetlist into a new PlaceHolderVar if it is a lateral reference to something outside the subquery. This is necessary when the Var/PHV references the non-nullable side of the outer join from the nullable side: we need to ensure that it is evaluated at the right place and hence is forced to null when the outer join should do so. However, if the referenced rel is under the same lowest nulling outer join, we can actually omit the wrapping. That's safe because if the subquery variable is forced to NULL by the outer join, the lateral reference variable will come out as NULL too. It could be beneficial to get rid of such PHVs because they imply lateral dependencies, which force us to resort to nestloop joins. This patch leverages the newly introduced nullingrel_info to check if the nullingrels of the subquery RTE are a subset of those of the laterally referenced rel, in order to determine if the referenced rel is under the same lowest nulling outer join. No backpatch as this could result in plan changes. Author: Richard Guo Reviewed-by: James Coleman, Dmitry Dolgov Discussion: https://postgr.es/m/CAMbWs48uk6C7Z9m_FNT8_21CMCk68hrgAsz=z6zpP1PNZMkeoQ@mail.gmail.com --- src/backend/optimizer/prep/prepjointree.c | 39 ++++-- src/test/regress/expected/subselect.out | 138 ++++++++++++++++++++++ src/test/regress/sql/subselect.sql | 36 ++++++ 3 files changed, 204 insertions(+), 9 deletions(-) diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 2ebd938f6b..3fa4d78c3e 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -2598,26 +2598,47 @@ pullup_replace_vars_callback(Var *var, /* * Simple Vars always escape being wrapped, unless they are * lateral references to something outside the subquery being - * pulled up. (Even then, we could omit the PlaceHolderVar if - * the referenced rel is under the same lowest outer join, but - * it doesn't seem worth the trouble to check that.) + * pulled up and the referenced rel is not under the same + * lowest nulling outer join. */ + wrap = false; if (rcon->target_rte->lateral && !bms_is_member(((Var *) newnode)->varno, rcon->relids)) - wrap = true; - else - wrap = false; + { + nullingrel_info *nullinfo = rcon->nullinfo; + int lvarno = ((Var *) newnode)->varno; + + Assert(lvarno > 0 && lvarno <= nullinfo->rtlength); + if (!bms_is_subset(nullinfo->nullingrels[rcon->varno], + nullinfo->nullingrels[lvarno])) + wrap = true; + } } else if (newnode && IsA(newnode, PlaceHolderVar) && ((PlaceHolderVar *) newnode)->phlevelsup == 0) { /* The same rules apply for a PlaceHolderVar */ + wrap = false; if (rcon->target_rte->lateral && !bms_is_subset(((PlaceHolderVar *) newnode)->phrels, rcon->relids)) - wrap = true; - else - wrap = false; + { + nullingrel_info *nullinfo = rcon->nullinfo; + Relids lvarnos = ((PlaceHolderVar *) newnode)->phrels; + int lvarno; + + lvarno = -1; + while ((lvarno = bms_next_member(lvarnos, lvarno)) >= 0) + { + Assert(lvarno > 0 && lvarno <= nullinfo->rtlength); + if (!bms_is_subset(nullinfo->nullingrels[rcon->varno], + nullinfo->nullingrels[lvarno])) + { + wrap = true; + break; + } + } + } } else if (rcon->wrap_non_vars) { diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index 6c1fb2bfdb..566dec67fa 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -1848,6 +1848,144 @@ order by 1, 2; 4567890123456789 | 9135780246913578 (11 rows) +-- lateral references for simple Vars can escape being wrapped if the +-- referenced rel is under the same lowest nulling outer join +explain (verbose, costs off) +select t1.q1, x from + int8_tbl t1 left join + (int8_tbl t2 inner join + lateral (select t2.q2 as x, * from int8_tbl t3) ss on t2.q2 = ss.q1) + on t1.q1 = t2.q1 +order by 1, 2; + QUERY PLAN +-------------------------------------------------------- + Sort + Output: t1.q1, t2.q2 + Sort Key: t1.q1, t2.q2 + -> Hash Right Join + Output: t1.q1, t2.q2 + Hash Cond: (t2.q1 = t1.q1) + -> Hash Join + Output: t2.q2, t2.q1 + Hash Cond: (t2.q2 = t3.q1) + -> Seq Scan on public.int8_tbl t2 + Output: t2.q1, t2.q2 + -> Hash + Output: t3.q1 + -> Seq Scan on public.int8_tbl t3 + Output: t3.q1 + -> Hash + Output: t1.q1 + -> Seq Scan on public.int8_tbl t1 + Output: t1.q1 +(19 rows) + +select t1.q1, x from + int8_tbl t1 left join + (int8_tbl t2 inner join + lateral (select t2.q2 as x, * from int8_tbl t3) ss on t2.q2 = ss.q1) + on t1.q1 = t2.q1 +order by 1, 2; + q1 | x +------------------+------------------ + 123 | 4567890123456789 + 123 | 4567890123456789 + 123 | 4567890123456789 + 123 | 4567890123456789 + 123 | 4567890123456789 + 123 | 4567890123456789 + 4567890123456789 | 123 + 4567890123456789 | 123 + 4567890123456789 | 123 + 4567890123456789 | 123 + 4567890123456789 | 123 + 4567890123456789 | 123 + 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 +(21 rows) + +-- lateral references for PHVs can also escape being wrapped if the +-- referenced rel is under the same lowest nulling outer join +explain (verbose, costs off) +select ss2.* from + int8_tbl t1 left join + (int8_tbl t2 left join + (select coalesce(q1) as x, * from int8_tbl t3) ss1 on t2.q1 = ss1.q2 inner join + lateral (select ss1.x as y, * from int8_tbl t4) ss2 on t2.q2 = ss2.q1) + on t1.q2 = ss2.q1 +order by 1, 2, 3; + QUERY PLAN +---------------------------------------------------------------- + Sort + Output: (COALESCE(t3.q1)), t4.q1, t4.q2 + Sort Key: (COALESCE(t3.q1)), t4.q1, t4.q2 + -> Hash Right Join + Output: (COALESCE(t3.q1)), t4.q1, t4.q2 + Hash Cond: (t4.q1 = t1.q2) + -> Hash Join + Output: (COALESCE(t3.q1)), t4.q1, t4.q2 + Hash Cond: (t2.q2 = t4.q1) + -> Hash Left Join + Output: t2.q2, (COALESCE(t3.q1)) + Hash Cond: (t2.q1 = t3.q2) + -> Seq Scan on public.int8_tbl t2 + Output: t2.q1, t2.q2 + -> Hash + Output: t3.q2, (COALESCE(t3.q1)) + -> Seq Scan on public.int8_tbl t3 + Output: t3.q2, COALESCE(t3.q1) + -> Hash + Output: t4.q1, t4.q2 + -> Seq Scan on public.int8_tbl t4 + Output: t4.q1, t4.q2 + -> Hash + Output: t1.q2 + -> Seq Scan on public.int8_tbl t1 + Output: t1.q2 +(26 rows) + +select ss2.* from + int8_tbl t1 left join + (int8_tbl t2 left join + (select coalesce(q1) as x, * from int8_tbl t3) ss1 on t2.q1 = ss1.q2 inner join + lateral (select ss1.x as y, * from int8_tbl t4) ss2 on t2.q2 = ss2.q1) + on t1.q2 = ss2.q1 +order by 1, 2, 3; + y | q1 | q2 +------------------+------------------+------------------- + 123 | 123 | 456 + 123 | 123 | 4567890123456789 + 123 | 4567890123456789 | -4567890123456789 + 123 | 4567890123456789 | -4567890123456789 + 123 | 4567890123456789 | 123 + 123 | 4567890123456789 | 123 + 123 | 4567890123456789 | 4567890123456789 + 123 | 4567890123456789 | 4567890123456789 + 4567890123456789 | 123 | 456 + 4567890123456789 | 123 | 4567890123456789 + 4567890123456789 | 4567890123456789 | -4567890123456789 + 4567890123456789 | 4567890123456789 | -4567890123456789 + 4567890123456789 | 4567890123456789 | -4567890123456789 + 4567890123456789 | 4567890123456789 | -4567890123456789 + 4567890123456789 | 4567890123456789 | 123 + 4567890123456789 | 4567890123456789 | 123 + 4567890123456789 | 4567890123456789 | 123 + 4567890123456789 | 4567890123456789 | 123 + 4567890123456789 | 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 | 4567890123456789 + 4567890123456789 | 4567890123456789 | 4567890123456789 + | | + | | +(24 rows) + -- -- Tests for CTE inlining behavior -- diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index e5a562c3f5..23c6350d94 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -939,6 +939,42 @@ select t1.q1, x from on t1.q2 = t2.q2 order by 1, 2; +-- lateral references for simple Vars can escape being wrapped if the +-- referenced rel is under the same lowest nulling outer join +explain (verbose, costs off) +select t1.q1, x from + int8_tbl t1 left join + (int8_tbl t2 inner join + lateral (select t2.q2 as x, * from int8_tbl t3) ss on t2.q2 = ss.q1) + on t1.q1 = t2.q1 +order by 1, 2; + +select t1.q1, x from + int8_tbl t1 left join + (int8_tbl t2 inner join + lateral (select t2.q2 as x, * from int8_tbl t3) ss on t2.q2 = ss.q1) + on t1.q1 = t2.q1 +order by 1, 2; + +-- lateral references for PHVs can also escape being wrapped if the +-- referenced rel is under the same lowest nulling outer join +explain (verbose, costs off) +select ss2.* from + int8_tbl t1 left join + (int8_tbl t2 left join + (select coalesce(q1) as x, * from int8_tbl t3) ss1 on t2.q1 = ss1.q2 inner join + lateral (select ss1.x as y, * from int8_tbl t4) ss2 on t2.q2 = ss2.q1) + on t1.q2 = ss2.q1 +order by 1, 2, 3; + +select ss2.* from + int8_tbl t1 left join + (int8_tbl t2 left join + (select coalesce(q1) as x, * from int8_tbl t3) ss1 on t2.q1 = ss1.q2 inner join + lateral (select ss1.x as y, * from int8_tbl t4) ss2 on t2.q2 = ss2.q1) + on t1.q2 = ss2.q1 +order by 1, 2, 3; + -- -- Tests for CTE inlining behavior -- -- 2.43.0 ^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: An inefficient query caused by unnecessary PlaceHolderVar @ 2024-12-03 08:33 Andrei Lepikhov <[email protected]> parent: Richard Guo <[email protected]> 0 siblings, 1 reply; 26+ messages in thread From: Andrei Lepikhov @ 2024-12-03 08:33 UTC (permalink / raw) To: Richard Guo <[email protected]>; Dmitry Dolgov <[email protected]>; +Cc: James Coleman <[email protected]>; pgsql-hackers On 12/2/24 10:46, Richard Guo wrote: > On Wed, Nov 27, 2024 at 5:45 PM Richard Guo <[email protected]> wrote: >> I ended up using 'under the same lowest nulling outer join' to >> keep consistent with the wording used elsewhere. Please see the >> updated patch attached. > > Commit e032e4c7d computes the nullingrel data for each leaf RTE, and > we can leverage that to determine if the referenced rel is under the > same lowest nulling outer join: we just need to check if the > nullingrels of the subquery RTE are a subset of those of the lateral > referenced rel. This eliminates the need to introduce > lowest_nullable_side. Please see attached. Thanks for drawing attention to e032e4c7d. It is a really helpful structure. I remember last year, we discussed [1] one sophisticated subquery pull-up technique, and we needed exactly the same data - it was too invasive to commit, and we committed only a small part of it. The nullingrel_info structure may give this feature one more chance. A couple of words about your patch. These few lines of code caused a lot of discoveries, but in my opinion, they look fine. But I didn't find negative tests, where we need to wrap a Var with PHV like the following: explain (verbose, costs off) select t1.q1, x from int8_tbl t1 left join (int8_tbl t2 left join lateral (select t2.q2 as x, * from int8_tbl t3) ss on t2.q2 = ss.q1) on t1.q1 = t2.q1 order by 1, 2; If regression tests doesn't contain such check it would be nice to add. [1] https://www.postgresql.org/message-id/35c8a3e8-d080-dfa8-2be3-cf5fe702010a%40postgrespro.ru -- regards, Andrei Lepikhov ^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: An inefficient query caused by unnecessary PlaceHolderVar @ 2024-12-09 12:20 Richard Guo <[email protected]> parent: Andrei Lepikhov <[email protected]> 0 siblings, 0 replies; 26+ messages in thread From: Richard Guo @ 2024-12-09 12:20 UTC (permalink / raw) To: Andrei Lepikhov <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; James Coleman <[email protected]>; pgsql-hackers On Tue, Dec 3, 2024 at 5:33 PM Andrei Lepikhov <[email protected]> wrote: > A couple of words about your patch. These few lines of code caused a lot > of discoveries, but in my opinion, they look fine. But I didn't find > negative tests, where we need to wrap a Var with PHV like the following: Pushed after adding two negative tests. Thank you for your review. Thanks Richard ^ permalink raw reply [nested|flat] 26+ messages in thread
end of thread, other threads:[~2024-12-09 12:20 UTC | newest] Thread overview: 26+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-12-01 23:20 [PATCH v8 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v10 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2020-12-01 23:20 [PATCH v9 4/4] WIP: check tuple size Justin Pryzby <[email protected]> 2024-12-02 03:46 Re: An inefficient query caused by unnecessary PlaceHolderVar Richard Guo <[email protected]> 2024-12-03 08:33 ` Re: An inefficient query caused by unnecessary PlaceHolderVar Andrei Lepikhov <[email protected]> 2024-12-09 12:20 ` Re: An inefficient query caused by unnecessary PlaceHolderVar Richard Guo <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox