mirror of https://github.com/citusdata/citus.git
Fix getting heap tuple size (#7387)
This fixes #7230. First of all, using HeapTupleHeaderGetDatumLength(heapTuple) is definetly wrong, it gives a number that's 4 times less than the correct tuple size (heapTuple.t_len). See https://github.com/postgres/postgres/blob/REL_16_0/src/include/access/htup_details.h#L455-L456 https://github.com/postgres/postgres/blob/REL_16_0/src/include/varatt.h#L279 https://github.com/postgres/postgres/blob/REL_16_0/src/include/varatt.h#L225-L226 When I fixed it, the limit_intermediate_size test failed, so I tried to understand what's going on there. In original commitpull/7840/headfd546cf
these queries were supposed to fail. Then inb3af63c
three of the queries that were supposed to fail suddenly worked and tests were changed to pass without understanding why the output had changed or how to keep test testing what it had to test. Even comments saying that these queries should fail were left untouched. Commit message gives no clue about why exactly test has changed: > It seems that when we use adaptive executor instead of task tracker, we > exceed the intermediate result size less in the test. Therefore updated > the tests accordingly. Then3fda2c3
also blindly raised the limit for one of the queries to keep it working:3fda2c3254 (diff-a9b7b617f9dfd345318cb8987d5897143ca1b723c87b81049bbadd94dcc86570R19)
When infe3caf3
that HeapTupleHeaderGetDatumLength(heapTuple) call was finally added, one of those test queries became failing again. The other two of them now also failing after the fix. I don't understand how exactly the calculation of "intermediate result size" that is limited by citus.max_intermediate_result_size had changed throughb3af63c
andfe3caf3
, but these numbers are now closer to what they originally were when this limitation was added infd546cf
. So these queries should fail, like in the original version of the limit_intermediate_size test. Co-authored-by: Karina Litskevich <litskevichkarina@gmail.com> (cherry picked from commit20dc58cf5d
)
parent
1a8349aeaf
commit
01945bd3a3
|
@ -109,7 +109,7 @@ TupleStoreTupleDestPutTuple(TupleDestination *self, Task *task,
|
|||
uint64 tupleSize = tupleLibpqSize;
|
||||
if (tupleSize == 0)
|
||||
{
|
||||
tupleSize = HeapTupleHeaderGetDatumLength(heapTuple);
|
||||
tupleSize = heapTuple->t_len;
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
|
@ -184,6 +184,7 @@ DEPS = {
|
|||
"foreign_key_to_reference_shard_rebalance": TestDeps(
|
||||
"minimal_schedule", ["remove_coordinator_from_metadata"]
|
||||
),
|
||||
"limit_intermediate_size": TestDeps("base_schedule"),
|
||||
}
|
||||
|
||||
|
||||
|
|
|
@ -16,7 +16,8 @@ SELECT cte.user_id, cte.value_2 FROM cte,cte2 ORDER BY 1,2 LIMIT 10;
|
|||
ERROR: the intermediate result size exceeds citus.max_intermediate_result_size (currently 2 kB)
|
||||
DETAIL: Citus restricts the size of intermediate results of complex subqueries and CTEs to avoid accidentally pulling large result sets into once place.
|
||||
HINT: To run the current query, set citus.max_intermediate_result_size to a higher value or -1 to disable.
|
||||
SET citus.max_intermediate_result_size TO 17;
|
||||
SET citus.max_intermediate_result_size TO 9;
|
||||
-- regular adaptive executor CTE should fail
|
||||
WITH cte AS MATERIALIZED
|
||||
(
|
||||
SELECT
|
||||
|
@ -38,20 +39,9 @@ FROM
|
|||
ORDER BY
|
||||
1,2
|
||||
LIMIT 10;
|
||||
user_id | value_2
|
||||
---------------------------------------------------------------------
|
||||
1 | 0
|
||||
1 | 0
|
||||
1 | 0
|
||||
1 | 0
|
||||
1 | 0
|
||||
1 | 0
|
||||
1 | 0
|
||||
1 | 0
|
||||
1 | 0
|
||||
1 | 0
|
||||
(10 rows)
|
||||
|
||||
ERROR: the intermediate result size exceeds citus.max_intermediate_result_size (currently 9 kB)
|
||||
DETAIL: Citus restricts the size of intermediate results of complex subqueries and CTEs to avoid accidentally pulling large result sets into once place.
|
||||
HINT: To run the current query, set citus.max_intermediate_result_size to a higher value or -1 to disable.
|
||||
-- router queries should be able to get limitted too
|
||||
SET citus.max_intermediate_result_size TO 2;
|
||||
-- this should pass, since we fetch small portions in each subplan
|
||||
|
@ -117,11 +107,9 @@ WITH cte AS MATERIALIZED (
|
|||
AND EXISTS (select * from cte2, cte3)
|
||||
)
|
||||
SELECT count(*) FROM cte WHERE EXISTS (select * from cte);
|
||||
count
|
||||
---------------------------------------------------------------------
|
||||
105
|
||||
(1 row)
|
||||
|
||||
ERROR: the intermediate result size exceeds citus.max_intermediate_result_size (currently 4 kB)
|
||||
DETAIL: Citus restricts the size of intermediate results of complex subqueries and CTEs to avoid accidentally pulling large result sets into once place.
|
||||
HINT: To run the current query, set citus.max_intermediate_result_size to a higher value or -1 to disable.
|
||||
SET citus.max_intermediate_result_size TO 3;
|
||||
-- this should fail since the cte-subplan exceeds the limit even if the
|
||||
-- cte2 and cte3 does not
|
||||
|
|
|
@ -17,7 +17,8 @@ cte2 AS MATERIALIZED (
|
|||
SELECT cte.user_id, cte.value_2 FROM cte,cte2 ORDER BY 1,2 LIMIT 10;
|
||||
|
||||
|
||||
SET citus.max_intermediate_result_size TO 17;
|
||||
SET citus.max_intermediate_result_size TO 9;
|
||||
-- regular adaptive executor CTE should fail
|
||||
WITH cte AS MATERIALIZED
|
||||
(
|
||||
SELECT
|
||||
|
|
Loading…
Reference in New Issue