Skip to content

Commit 8ffa0f4

Browse files
authored
Add a debug GUC to force batch sorted merge plan (#8384)
At the moment a significant part of the tests we have for batch sorted merge relies on the completely broken cost and size estimations for plain DecompressChunk. This prevents actually fixing these estimates. At the same time, it is unfeasible to rewrite these tests entirely. Add a GUC to force the batch sorted merge plan regardless of the cost, so that we can test the correctness on small tables.
1 parent fdc97ef commit 8ffa0f4

10 files changed

Lines changed: 292 additions & 218 deletions

src/guc.c

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,6 @@ TSDLLEXPORT char *ts_guc_license = TS_LICENSE_DEFAULT;
144144
char *ts_last_tune_time = NULL;
145145
char *ts_last_tune_version = NULL;
146146

147-
bool ts_guc_debug_require_batch_sorted_merge = false;
148-
149147
bool ts_guc_debug_allow_cagg_with_deprecated_funcs = false;
150148

151149
/*
@@ -169,20 +167,23 @@ char *ts_current_timestamp_mock = NULL;
169167

170168
int ts_guc_debug_toast_tuple_target = 128;
171169

172-
#ifdef TS_DEBUG
173-
174-
bool ts_guc_debug_have_int128;
175-
176170
static const struct config_enum_entry debug_require_options[] = { { "allow", DRO_Allow, false },
177171
{ "forbid", DRO_Forbid, false },
178172
{ "require", DRO_Require, false },
173+
{ "force", DRO_Force, false },
179174
{ NULL, 0, false } };
180175

176+
#ifdef TS_DEBUG
177+
178+
bool ts_guc_debug_have_int128;
179+
181180
DebugRequireOption ts_guc_debug_require_vector_qual = DRO_Allow;
182181

183182
DebugRequireOption ts_guc_debug_require_vector_agg = DRO_Allow;
184183
#endif
185184

185+
DebugRequireOption ts_guc_debug_require_batch_sorted_merge = false;
186+
186187
bool ts_guc_debug_compression_path_info = false;
187188
bool ts_guc_enable_rowlevel_compression_locking = false;
188189

@@ -1189,6 +1190,18 @@ _guc_init(void)
11891190
/* assign_hook= */ NULL,
11901191
/* show_hook= */ NULL);
11911192

1193+
DefineCustomEnumVariable(/* name= */ MAKE_EXTOPTION("debug_require_batch_sorted_merge"),
1194+
/* short_desc= */ "require batch sorted merge in DecompressChunk node",
1195+
/* long_desc= */ "this is for debugging purposes",
1196+
/* valueAddr= */ (int *) &ts_guc_debug_require_batch_sorted_merge,
1197+
/* bootValue= */ DRO_Allow,
1198+
/* options = */ debug_require_options,
1199+
/* context= */ PGC_USERSET,
1200+
/* flags= */ 0,
1201+
/* check_hook= */ NULL,
1202+
/* assign_hook= */ NULL,
1203+
/* show_hook= */ NULL);
1204+
11921205
#ifdef TS_DEBUG
11931206
DefineCustomBoolVariable(/* name= */ MAKE_EXTOPTION("shutdown_bgw_scheduler"),
11941207
/* short_desc= */ "immediately shutdown the bgw scheduler",
@@ -1271,17 +1284,6 @@ _guc_init(void)
12711284
/* assign_hook= */ NULL,
12721285
/* show_hook= */ NULL);
12731286

1274-
DefineCustomBoolVariable(/* name= */ MAKE_EXTOPTION("debug_require_batch_sorted_merge"),
1275-
/* short_desc= */ "require batch sorted merge in DecompressChunk node",
1276-
/* long_desc= */ "this is for debugging purposes",
1277-
/* valueAddr= */ &ts_guc_debug_require_batch_sorted_merge,
1278-
/* bootValue= */ false,
1279-
/* context= */ PGC_USERSET,
1280-
/* flags= */ 0,
1281-
/* check_hook= */ NULL,
1282-
/* assign_hook= */ NULL,
1283-
/* show_hook= */ NULL);
1284-
12851287
DefineCustomBoolVariable(/* name= */ MAKE_EXTOPTION("debug_allow_cagg_with_deprecated_funcs"),
12861288
/* short_desc= */ "allow new caggs using time_bucket_ng",
12871289
/* long_desc= */ "this is for debugging/testing purposes",

src/guc.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,15 @@ extern char *ts_current_timestamp_mock;
119119

120120
extern TSDLLEXPORT int ts_guc_debug_toast_tuple_target;
121121

122-
#ifdef TS_DEBUG
123122
typedef enum DebugRequireOption
124123
{
125124
DRO_Allow = 0,
126125
DRO_Forbid,
127-
DRO_Require
126+
DRO_Require,
127+
DRO_Force,
128128
} DebugRequireOption;
129129

130+
#ifdef TS_DEBUG
130131
extern TSDLLEXPORT DebugRequireOption ts_guc_debug_require_vector_qual;
131132

132133
extern TSDLLEXPORT DebugRequireOption ts_guc_debug_require_vector_agg;
@@ -136,7 +137,7 @@ extern TSDLLEXPORT DebugRequireOption ts_guc_debug_require_vector_agg;
136137
extern TSDLLEXPORT bool ts_guc_debug_compression_path_info;
137138
extern TSDLLEXPORT bool ts_guc_enable_rowlevel_compression_locking;
138139

139-
extern TSDLLEXPORT bool ts_guc_debug_require_batch_sorted_merge;
140+
extern TSDLLEXPORT DebugRequireOption ts_guc_debug_require_batch_sorted_merge;
140141

141142
extern TSDLLEXPORT bool ts_guc_debug_allow_cagg_with_deprecated_funcs;
142143

tsl/src/nodes/decompress_chunk/decompress_chunk.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -990,13 +990,27 @@ ts_decompress_chunk_generate_paths(PlannerInfo *root, RelOptInfo *chunk_rel, con
990990
batch_merge_path->custom_path.path.pathkeys = root->query_pathkeys;
991991
cost_batch_sorted_merge(root, compression_info, batch_merge_path, compressed_path);
992992

993+
if (ts_guc_debug_require_batch_sorted_merge == DRO_Force)
994+
{
995+
batch_merge_path->custom_path.path.startup_cost = cpu_tuple_cost;
996+
batch_merge_path->custom_path.path.total_cost = 2 * cpu_tuple_cost;
997+
}
998+
993999
/* If the chunk is partially compressed, prepare the path only and add it later
9941000
* to a merge append path when we are able to generate the ordered result for the
9951001
* compressed and uncompressed part of the chunk.
9961002
*/
9971003
if (!consider_partial)
9981004
add_path(chunk_rel, &batch_merge_path->custom_path.path);
9991005
}
1006+
else if (ts_guc_debug_require_batch_sorted_merge == DRO_Require ||
1007+
ts_guc_debug_require_batch_sorted_merge == DRO_Force)
1008+
{
1009+
ereport(ERROR,
1010+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
1011+
errmsg("debug: batch sorted merge is required but not possible at planning "
1012+
"time")));
1013+
}
10001014

10011015
/* If we can push down the sort below the DecompressChunk node, we set the pathkeys of
10021016
* the decompress node to the query pathkeys, while remembering the compressed_pathkeys

tsl/src/nodes/decompress_chunk/exec.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,13 +383,22 @@ decompress_chunk_begin(CustomScanState *node, EState *estate, int eflags)
383383
chunk_state->exec_methods.ExecCustomScan = decompress_chunk_exec_fifo;
384384
}
385385

386-
if (ts_guc_debug_require_batch_sorted_merge && !dcontext->batch_sorted_merge)
386+
if ((ts_guc_debug_require_batch_sorted_merge == DRO_Require ||
387+
ts_guc_debug_require_batch_sorted_merge == DRO_Force) &&
388+
!dcontext->batch_sorted_merge)
387389
{
388390
ereport(ERROR,
389391
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
390392
errmsg("debug: batch sorted merge is required but not used")));
391393
}
392394

395+
if (ts_guc_debug_require_batch_sorted_merge == DRO_Forbid && dcontext->batch_sorted_merge)
396+
{
397+
ereport(ERROR,
398+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
399+
errmsg("debug: batch sorted merge is used when it is forbidden")));
400+
}
401+
393402
/* Constify stable expressions in vectorized predicates. */
394403
PlannerGlobal glob = {
395404
.boundParams = node->ss.ps.state->es_param_list_info,

0 commit comments

Comments
 (0)