diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index 2abeb92c2eb933fb8ad7e1e53dd2d3f5ca41c6da..e433fb6ca325b658a1079a6b820cafdecc5875a7 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -298,29 +298,16 @@ transformRelOptions(Datum oldOptions, List *defList, * have just "name", assume "name=true" is meant. */ - bool need_free_value = false; if (def->arg != NULL) - { - value = defGetString(def, &need_free_value); - } + value = defGetString(def); else - { value = "true"; - } len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value); /* +1 leaves room for sprintf's trailing null */ t = (text *) palloc(len + 1); SET_VARSIZE(t, len); sprintf(VARDATA(t), "%s=%s", def->defname, value); - if (need_free_value) - { - pfree(value); - value = NULL; - } - - AssertImply(need_free_value, NULL == value); - astate = accumArrayResult(astate, PointerGetDatum(t), false, TEXTOID, CurrentMemoryContext); diff --git a/src/backend/commands/aggregatecmds.c b/src/backend/commands/aggregatecmds.c index 8ec8443369a6f50312a7080f1e3319d31586cbf1..4085207a05e12c4d4a68794c5b70ceefe504b9f6 100644 --- a/src/backend/commands/aggregatecmds.c +++ b/src/backend/commands/aggregatecmds.c @@ -67,7 +67,6 @@ DefineAggregate(List *name, List *args, bool oldstyle, List *parameters, Oid transTypeId; ListCell *pl; Oid aggOid; - bool need_free_value = false; /* Convert list of names to a name and namespace */ aggNamespace = QualifiedNameGetCreationNamespace(name, &aggName); @@ -101,9 +100,9 @@ DefineAggregate(List *name, List *args, bool oldstyle, List *parameters, else if (pg_strcasecmp(defel->defname, "stype1") == 0) transType = defGetTypeName(defel); else if (pg_strcasecmp(defel->defname, "initcond") == 0) - initval = defGetString(defel, &need_free_value); + initval = defGetString(defel); else if (pg_strcasecmp(defel->defname, "initcond1") == 0) - initval = defGetString(defel, &need_free_value); + initval = defGetString(defel); else if (pg_strcasecmp(defel->defname, "prefunc") == 0) /* MPP */ prelimfuncName = defGetQualifiedName(defel); else diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c index 882b91fa1c465a5d9a1ebd22d61be5cd127f147e..981352be08f59d1da3eb86756c86bcfaca0de73b 100644 --- a/src/backend/commands/define.c +++ b/src/backend/commands/define.c @@ -59,14 +59,13 @@ case_translate_language_name(const char *input) * Extract a string value (otherwise uninterpreted) from a DefElem. */ char * -defGetString(DefElem *def, bool *need_free) +defGetString(DefElem *def) { if (def->arg == NULL) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("%s requires a parameter", def->defname))); - Assert(NULL != need_free); switch (nodeTag(def->arg)) { case T_Integer: @@ -74,7 +73,6 @@ defGetString(DefElem *def, bool *need_free) char *str = palloc(32); snprintf(str, 32, "%ld", (long) intVal(def->arg)); - *need_free = true; return str; } case T_Float: @@ -83,16 +81,12 @@ defGetString(DefElem *def, bool *need_free) * T_Float values are kept in string form, so this type cheat * works (and doesn't risk losing precision) */ - *need_free = false; return strVal(def->arg); case T_String: - *need_free = false; return strVal(def->arg); case T_TypeName: - *need_free = true; return TypeNameToString((TypeName *) def->arg); case T_List: - *need_free = true; return NameListToString((List *) def->arg); default: elog(ERROR, "unrecognized node type: %d", (int) nodeTag(def->arg)); @@ -157,26 +151,13 @@ defGetBoolean(DefElem *def) break; default: { - bool need_free_value = false; - char *sval = defGetString(def, &need_free_value); - bool result = false; + char *sval = defGetString(def); + if (pg_strcasecmp(sval, "true") == 0) - { - result = true; - } + return true; if (pg_strcasecmp(sval, "false") == 0) - { - result = false; - } - if (need_free_value) - { - pfree(sval); - sval = NULL; - } - - AssertImply(need_free_value, NULL == sval); + return false; - return result; } break; } @@ -317,11 +298,10 @@ defGetTypeLength(DefElem *def) default: elog(ERROR, "unrecognized node type: %d", (int) nodeTag(def->arg)); } - bool need_free_value = false; ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("invalid argument for %s: \"%s\"", - def->defname, defGetString(def, &need_free_value)))); + def->defname, defGetString(def)))); return 0; /* keep compiler quiet */ } diff --git a/src/backend/commands/queue.c b/src/backend/commands/queue.c index b03ac3c34125e78eb59b2483b52ea60b6bd0bae8..9b3e47c1a70f2c51cae499a3a7bbeafc41ff25f6 100644 --- a/src/backend/commands/queue.c +++ b/src/backend/commands/queue.c @@ -347,10 +347,7 @@ AlterResqueueCapabilityEntry( /* WITHOUT clause value determined in pg_resourcetype */ if (!bWithout) - { - bool need_free_value = false; - pStrVal = makeString(defGetString(defel, &need_free_value)); - } + pStrVal = makeString(defGetString(defel)); else { pStrVal = NULL; /* if NULL, delete entry from diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index eb0adc8f08425087290b8e2230cdfce4da000989..d05fd73c645fd8b94c3e30bb14b6bc26fb835a78 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12206,7 +12206,6 @@ new_rel_opts(Relation rel, List *lwith) { Datum newOptions = PointerGetDatum(NULL); bool make_heap = false; - bool need_free_value = false; if (lwith && list_length(lwith)) { ListCell *lc; @@ -12219,7 +12218,7 @@ new_rel_opts(Relation rel, List *lwith) { DefElem *e = lfirst(lc); if (pg_strcasecmp(e->defname, "appendonly") == 0 && - pg_strcasecmp(defGetString(e, &need_free_value), "false") == 0) + pg_strcasecmp(defGetString(e), "false") == 0) { make_heap = true; break; @@ -17922,8 +17921,7 @@ static Datum transformFormatOpts(char formattype, List *formatOpts, int numcols, { DefElem *defel = (DefElem *) lfirst(option); char *key = defel->defname; - bool need_free_value = false; - char *val = defGetString(defel, &need_free_value); + char *val = defGetString(defel); if (strcmp(key, "formatter") == 0) { @@ -17934,22 +17932,14 @@ static Datum transformFormatOpts(char formattype, List *formatOpts, int numcols, formatter = strVal(defel->arg); } - + /* MPP-14467 - replace any space chars with meta char */ resetStringInfo(&key_modified); appendStringInfoString(&key_modified, key); replaceStringInfoString(&key_modified, " ", ""); - + sprintf((char *) format_str + len, "%s '%s' ", key_modified.data, val); len += strlen(key_modified.data) + strlen(val) + 4; - - if (need_free_value) - { - pfree(val); - val = NULL; - } - - AssertImply(need_free_value, NULL == val); if (len > maxlen) ereport(ERROR, diff --git a/src/backend/commands/test/Makefile b/src/backend/commands/test/Makefile index cefe444c3a6571f400017bf57d34ee1d0f3f257f..e0da0c50d796daffd9fa07cde6e5cf543f13d711 100644 --- a/src/backend/commands/test/Makefile +++ b/src/backend/commands/test/Makefile @@ -2,6 +2,6 @@ subdir=src/backend/commands top_builddir=../../../.. include $(top_builddir)/src/Makefile.global -TARGETS=tablecmds define +TARGETS=tablecmds include $(top_builddir)/src/backend/mock.mk diff --git a/src/backend/commands/test/define_test.c b/src/backend/commands/test/define_test.c deleted file mode 100644 index 80f31188cab8eeead8b5f9272bff946052f63657..0000000000000000000000000000000000000000 --- a/src/backend/commands/test/define_test.c +++ /dev/null @@ -1,72 +0,0 @@ -#include -#include -#include -#include "cmockery.h" - -#include "../define.c" - -/* ==================== FreeStrFromDefGetString ==================== */ - -/* - * Tests that need_free flag correctly set in defGetString method. - */ -void -test__DefGetString_NeedFreeFlag(void **state) -{ - bool need_free = false; - char *value = NULL; - - /* case: T_String expected value: false */ - need_free = true; - DefElem *e1 = makeDefElem("def_string", (Node *) makeString("none")); - value = defGetString(e1, &need_free); - assert_false(need_free); - - /* case: T_Integer expected value: true */ - need_free = false; - DefElem *e2 = makeDefElem("def_int", (Node *) makeInteger(0)); - value = defGetString(e2, &need_free); - assert_true(need_free); - - /* case: T_Float expected value: false */ - need_free = true; - DefElem *e3 = makeDefElem("def_float", (Node *) makeFloat("3.14")); - value = defGetString(e3, &need_free); - assert_false(need_free); - - /* case: T_TypeName expected value: true */ - need_free = false; - TypeName *tName = makeNode(TypeName); - tName->names = list_make2(makeString("pg_catalog"), makeString("unknown")); - tName->typemod = -1; - tName->location = -1; - DefElem *e4 = makeDefElem("def_typename", (Node *) tName); - value = defGetString(e4, &need_free); - assert_true(need_free); - - /* case: T_List expected value: true */ - need_free = false; - List *list = NIL; - list = lappend(list, makeString("str1")); - list = lappend(list, makeString("str2")); - DefElem *e5 = makeDefElem("def_list", (Node *) list); - value = defGetString(e5, &need_free); - assert_true(need_free); - -} - -/* ==================== main ==================== */ -int -main(int argc, char* argv[]) -{ - cmockery_parse_arguments(argc, argv); - - const UnitTest tests[] = { - unit_test(test__DefGetString_NeedFreeFlag) - }; - - MemoryContextInit(); - - return run_tests(tests); -} - diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index c788bee488f9911fa653e6078a4fec25578f6220..4b7c1e156a45beccc45e85bc126a03cbbd67c9e4 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -133,7 +133,6 @@ DefineType(List *names, List *parameters, Oid newOid, Oid shadowOid) Oid resulttype; Datum typoptions = 0; List *encoding = NIL; - bool need_free_value = false; Relation pg_type; /* Convert list of names to a name and namespace */ @@ -236,7 +235,7 @@ DefineType(List *names, List *parameters, Oid newOid, Oid shadowOid) analyzeName = defGetQualifiedName(defel); else if (pg_strcasecmp(defel->defname, "delimiter") == 0) { - char *p = defGetString(defel, &need_free_value); + char *p = defGetString(defel); delimiter = p[0]; } @@ -251,12 +250,12 @@ DefineType(List *names, List *parameters, Oid newOid, Oid shadowOid) format_type_be(elemType)))); } else if (pg_strcasecmp(defel->defname, "default") == 0) - defaultValue = defGetString(defel, &need_free_value); + defaultValue = defGetString(defel); else if (pg_strcasecmp(defel->defname, "passedbyvalue") == 0) byValue = defGetBoolean(defel); else if (pg_strcasecmp(defel->defname, "alignment") == 0) { - char *a = defGetString(defel, &need_free_value); + char *a = defGetString(defel); /* * Note: if argument was an unquoted identifier, parser will have @@ -283,7 +282,7 @@ DefineType(List *names, List *parameters, Oid newOid, Oid shadowOid) } else if (pg_strcasecmp(defel->defname, "storage") == 0) { - char *a = defGetString(defel, &need_free_value); + char *a = defGetString(defel); if (pg_strcasecmp(a, "plain") == 0) storage = 'p'; diff --git a/src/backend/gpopt/gpdbwrappers.cpp b/src/backend/gpopt/gpdbwrappers.cpp index 0d27d6fd4e9321fc4c27bfd91e69f11859b489b0..36eebc9dd69d04008a745a66c70185f2e10997f8 100644 --- a/src/backend/gpopt/gpdbwrappers.cpp +++ b/src/backend/gpopt/gpdbwrappers.cpp @@ -2969,13 +2969,12 @@ gpdb::FInterpretOidsOption char * gpdb::SzDefGetString ( - DefElem *pdefelem, - bool *fNeedFree + DefElem *pdefelem ) { GP_WRAP_START; { - return defGetString(pdefelem, fNeedFree); + return defGetString(pdefelem); } GP_WRAP_END; return NULL; diff --git a/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp b/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp index 11904f096b7f37beea1f4c7dd75e63a53643c0d8..f4d15185a580e8998ca371b89703ed12f7ef0254 100644 --- a/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp +++ b/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp @@ -1005,14 +1005,10 @@ CTranslatorQueryToDXL::PstrExtractOptionValue GPOS_ASSERT(NULL != pdefelem); BOOL fNeedsFree = false; - CHAR *szValue = gpdb::SzDefGetString(pdefelem, &fNeedsFree); + CHAR *szValue = gpdb::SzDefGetString(pdefelem); CWStringDynamic *pstrResult = CDXLUtils::PstrFromSz(m_pmp, szValue); - if (fNeedsFree) - { - gpdb::GPDBFree(szValue); - } return pstrResult; } diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index fa5d0182ad1ff4cda2d832bc8aac447f7e084157..c7a3a32af0e3c6eedda2e853f90a71cf680bf856 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -1298,8 +1298,7 @@ co_explicitly_disabled(List *opts) continue; } - bool need_free_arg = false; - arg = defGetString(el, &need_free_arg); + arg = defGetString(el); bool result = false; if (pg_strcasecmp("appendonly", el->defname) == 0 && pg_strcasecmp("false", arg) == 0) @@ -1311,13 +1310,6 @@ co_explicitly_disabled(List *opts) { result = true; } - if (need_free_arg) - { - pfree(arg); - arg = NULL; - } - - AssertImply(need_free_arg, NULL == arg); if (result) { @@ -1352,8 +1344,7 @@ is_aocs(List *opts) continue; } - bool need_free_arg = false; - arg = defGetString(el, &need_free_arg); + arg = defGetString(el); if (pg_strcasecmp("appendonly", el->defname) == 0) { @@ -1366,13 +1357,6 @@ is_aocs(List *opts) found_cs = true; csvalue = pg_strcasecmp("column", arg) == 0; } - if (need_free_arg) - { - pfree(arg); - arg = NULL; - } - - AssertImply(need_free_arg, NULL == arg); } if (!found_ao) aovalue = isDefaultAO(); @@ -1411,30 +1395,11 @@ encodings_overlap(List *a, List *b, bool test_conflicts) } else { - bool need_free_ela = false; - bool need_free_elb = false; - char *ela_str = defGetString(ela, &need_free_ela); - char *elb_str = defGetString(elb, &need_free_elb); - int result = pg_strcasecmp(ela_str,elb_str); - // free ela_str, elb_str if it is initialized via TypeNameToString - if (need_free_ela) - { - pfree(ela_str); - ela_str = NULL; - } - if (need_free_elb) - { - pfree(elb_str); - elb_str = NULL; - } + char *ela_str = defGetString(ela); + char *elb_str = defGetString(elb); - AssertImply(need_free_ela, NULL == ela_str); - AssertImply(need_free_elb, NULL == elb_str); - - if (result != 0) - { + if (pg_strcasecmp(ela_str, elb_str) != 0) return true; - } } } else @@ -3160,7 +3125,6 @@ fillin_encoding(List *list) bool foundCompressType = false; bool foundCompressTypeNone = false; char *cmplevel = NULL; - bool need_free_cmplevel = false; bool foundBlockSize = false; char *arg; List *retList = list; @@ -3175,21 +3139,13 @@ fillin_encoding(List *list) if (pg_strcasecmp("compresstype", el->defname) == 0) { foundCompressType = true; - bool need_free_arg = false; - arg = defGetString(el, &need_free_arg); + arg = defGetString(el); if (pg_strcasecmp("none", arg) == 0) foundCompressTypeNone = true; - if (need_free_arg) - { - pfree(arg); - arg = NULL; - } - - AssertImply(need_free_arg, NULL == arg); } else if (pg_strcasecmp("compresslevel", el->defname) == 0) { - cmplevel = defGetString(el, &need_free_cmplevel); + cmplevel = defGetString(el); } else if (pg_strcasecmp("blocksize", el->defname) == 0) foundBlockSize = true; diff --git a/src/backend/parser/parse_partition.c b/src/backend/parser/parse_partition.c index c5e009ef67707b2f32f3a95302191842bb44d8aa..1dfa8327b9ccfe56048aaf532a4bc26cb18339f7 100644 --- a/src/backend/parser/parse_partition.c +++ b/src/backend/parser/parse_partition.c @@ -717,17 +717,9 @@ transformPartitionBy(ParseState *pstate, CreateStmtContext *cxt, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("invalid tablename specification"))); - bool need_free_value = false; - char *relname_str = defGetString(pDef, &need_free_value); + char *relname_str = defGetString(pDef); relname = pstrdup(relname_str); - if (need_free_value) - { - pfree(relname_str); - relname_str = NULL; - } - - AssertImply(need_free_value, NULL == relname_str); prtstr[0] = '\0'; pWithList = list_delete_cell(pWithList, def_lc, prev_lc); @@ -3610,17 +3602,9 @@ partition_range_every(ParseState *pstate, PartitionBy *pBy, List *coltypes, errmsg("invalid tablename specification"))); bTablename = true; - bool need_free_value = false; - char *widthname_str = defGetString(pDef, &need_free_value); + char *widthname_str = defGetString(pDef); pBSpec->pWithTnameStr = pstrdup(widthname_str); - if (need_free_value) - { - pfree(widthname_str); - widthname_str = NULL; - } - - AssertImply(need_free_value, NULL == widthname_str); pWithList = list_delete_cell(pWithList, def_lc, prev_lc); ((AlterPartitionCmd *) pStoreAttr)->arg1 = diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index 188b83a61bd247ee21cd5efdb98e8b1e343b6e68..1062ee407c9f1ea8d3f9bfc5ba573afd2eb36cd7 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -101,7 +101,7 @@ extern void AlterOpFamilyOwner(List *name, const char *access_method, Oid newOwn extern char *case_translate_language_name(const char *input); -extern char *defGetString(DefElem *def, bool *need_free); +extern char *defGetString(DefElem *def); extern double defGetNumeric(DefElem *def); extern bool defGetBoolean(DefElem *def); extern int64 defGetInt64(DefElem *def); diff --git a/src/include/gpopt/gpdbwrappers.h b/src/include/gpopt/gpdbwrappers.h index 7473fe1f36ab248e477126be6fc2a758e20dad19..ee64f5ad53762f4bf7e3bc4a175ed13adde87559 100644 --- a/src/include/gpopt/gpdbwrappers.h +++ b/src/include/gpopt/gpdbwrappers.h @@ -596,7 +596,7 @@ namespace gpdb { bool FInterpretOidsOption(List *plOptions); // extract string value from defelem's value - char *SzDefGetString(DefElem *pdefelem, bool *fNeedFree); + char *SzDefGetString(DefElem *pdefelem); // fold array expression constant values Node *PnodeFoldArrayexprConstants(ArrayExpr *parrayexpr);