introduce TASK_QUERY_NULL task type (#3774)

When we call SetTaskQueryString we would set the task type to
TASK_QUERY_TEXT, and some parts of the codebase rely on the fact that if
TASK_QUERY_TEXT is set, the data can be read safely. However if
SetTaskQueryString is called with a NULL taskQueryString this can cause
crashes. In that case taskQueryType will simply be set to
TASK_QUERY_NULL.
pull/3757/head
SaitTalhaNisanci 2020-04-17 14:59:22 +03:00 committed by GitHub
parent 2d50f63841
commit a9a3be15cc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 73 additions and 8 deletions

View File

@ -443,8 +443,15 @@ SetTaskQueryIfShouldLazyDeparse(Task *task, Query *query)
void void
SetTaskQueryString(Task *task, char *queryString) SetTaskQueryString(Task *task, char *queryString)
{ {
if (queryString == NULL)
{
task->taskQuery.queryType = TASK_QUERY_NULL;
}
else
{
task->taskQuery.queryType = TASK_QUERY_TEXT; task->taskQuery.queryType = TASK_QUERY_TEXT;
task->taskQuery.data.queryStringLazy = queryString; task->taskQuery.data.queryStringLazy = queryString;
}
} }
@ -466,6 +473,7 @@ SetTaskPerPlacementQueryStrings(Task *task, List *perPlacementQueryStringList)
void void
SetTaskQueryStringList(Task *task, List *queryStringList) SetTaskQueryStringList(Task *task, List *queryStringList)
{ {
Assert(queryStringList != NIL);
task->taskQuery.queryType = TASK_QUERY_TEXT_LIST; task->taskQuery.queryType = TASK_QUERY_TEXT_LIST;
task->taskQuery.data.queryStringList = queryStringList; task->taskQuery.data.queryStringList = queryStringList;
} }
@ -520,16 +528,32 @@ GetTaskQueryType(Task *task)
char * char *
TaskQueryStringForAllPlacements(Task *task) TaskQueryStringForAllPlacements(Task *task)
{ {
if (GetTaskQueryType(task) == TASK_QUERY_TEXT_LIST) int taskQueryType = GetTaskQueryType(task);
if (taskQueryType == TASK_QUERY_NULL)
{
/* if task query type is TASK_QUERY_NULL then the data will be NULL,
* this is unexpected state */
ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR),
errmsg("unexpected task query state: task query type is null"),
errdetail("Please report this to the Citus core team.")));
}
else if (taskQueryType == TASK_QUERY_TEXT_LIST)
{ {
return StringJoin(task->taskQuery.data.queryStringList, ';'); return StringJoin(task->taskQuery.data.queryStringList, ';');
} }
if (GetTaskQueryType(task) == TASK_QUERY_TEXT) else if (taskQueryType == TASK_QUERY_TEXT)
{ {
return task->taskQuery.data.queryStringLazy; return task->taskQuery.data.queryStringLazy;
} }
Query *jobQueryReferenceForLazyDeparsing = Query *jobQueryReferenceForLazyDeparsing =
task->taskQuery.data.jobQueryReferenceForLazyDeparsing; task->taskQuery.data.jobQueryReferenceForLazyDeparsing;
/*
* At this point task query type should be TASK_QUERY_OBJECT.
* if someone calls this method inappropriately with TASK_QUERY_TEXT_PER_PLACEMENT case
* (instead of TaskQueryStringForPlacement), they will hit this assert.
*/
Assert(task->taskQuery.queryType == TASK_QUERY_OBJECT && Assert(task->taskQuery.queryType == TASK_QUERY_OBJECT &&
jobQueryReferenceForLazyDeparsing != NULL); jobQueryReferenceForLazyDeparsing != NULL);

View File

@ -205,6 +205,7 @@ typedef struct TaskExecution TaskExecution;
typedef enum TaskQueryType typedef enum TaskQueryType
{ {
TASK_QUERY_NULL,
TASK_QUERY_TEXT, TASK_QUERY_TEXT,
TASK_QUERY_OBJECT, TASK_QUERY_OBJECT,
TASK_QUERY_TEXT_PER_PLACEMENT, TASK_QUERY_TEXT_PER_PLACEMENT,

View File

@ -909,6 +909,37 @@ LOG: join order: [ "colocated_table_test" ][ reference join "reference_table_te
SET client_min_messages TO NOTICE; SET client_min_messages TO NOTICE;
SET citus.log_multi_join_order TO FALSE; SET citus.log_multi_join_order TO FALSE;
-- clean up tables
\c - - - :master_port \c - - - :master_port
DROP TABLE reference_table_test, reference_table_test_second, reference_table_test_third;; -- issue 3766
CREATE TABLE numbers(a int);
SELECT create_reference_table('numbers');
create_reference_table
---------------------------------------------------------------------
(1 row)
SET client_min_messages TO debug4;
INSERT INTO numbers VALUES (1), (2), (3), (4);
DEBUG: Creating router plan
DEBUG: Plan is router executable
DEBUG: query before rebuilding: (null)
DEBUG: query after rebuilding: INSERT INTO public.numbers_1250015 AS citus_table_alias (a) VALUES (1), (2), (3), (4)
DEBUG: assigned task to node localhost:xxxxx
DEBUG: opening 1 new connections to localhost:xxxxx
DEBUG: established connection to localhost:xxxxx for session 4
DEBUG: opening 1 new connections to localhost:xxxxx
DEBUG: established connection to localhost:xxxxx for session 5
DEBUG: Total number of commands sent over the session 4: 1
DEBUG: Total number of commands sent over the session 5: 1
SELECT count(*) FROM numbers;
DEBUG: Distributed planning for a fast-path router query
DEBUG: Creating router plan
DEBUG: Plan is router executable
count
---------------------------------------------------------------------
4
(1 row)
RESET client_min_messages;
-- clean up tables
DROP TABLE reference_table_test, reference_table_test_second, reference_table_test_third, numbers;

View File

@ -557,6 +557,15 @@ ORDER BY 1;
SET client_min_messages TO NOTICE; SET client_min_messages TO NOTICE;
SET citus.log_multi_join_order TO FALSE; SET citus.log_multi_join_order TO FALSE;
-- clean up tables
\c - - - :master_port \c - - - :master_port
DROP TABLE reference_table_test, reference_table_test_second, reference_table_test_third;;
-- issue 3766
CREATE TABLE numbers(a int);
SELECT create_reference_table('numbers');
SET client_min_messages TO debug4;
INSERT INTO numbers VALUES (1), (2), (3), (4);
SELECT count(*) FROM numbers;
RESET client_min_messages;
-- clean up tables
DROP TABLE reference_table_test, reference_table_test_second, reference_table_test_third, numbers;