You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hawq.apache.org by iw...@apache.org on 2016/08/11 02:24:47 UTC
incubator-hawq git commit: HAWQ-980. hawq does not handle guc value
with space properly
Repository: incubator-hawq
Updated Branches:
refs/heads/master ac00c0ece -> c742cd716
HAWQ-980. hawq does not handle guc value with space properly
This patch also includes some code change for feature_test framework.
Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/c742cd71
Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/c742cd71
Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/c742cd71
Branch: refs/heads/master
Commit: c742cd716819f177dc3951b777c26e2b278c4d40
Parents: ac00c0e
Author: Paul Guo <pa...@gmail.com>
Authored: Thu Aug 4 19:51:11 2016 +0800
Committer: ivan <iw...@pivotal.io>
Committed: Thu Aug 11 10:22:04 2016 +0800
----------------------------------------------------------------------
src/backend/cdb/executormgr.c | 41 +++++++++++-----
src/backend/postmaster/postmaster.c | 77 ++++++++++++++++++++----------
src/backend/utils/misc/guc.c | 46 ++++++++++++++++--
src/test/feature/.gitignore | 16 +++++++
src/test/feature/catalog/ans/guc.ans | 26 ++++++++++
src/test/feature/catalog/sql/guc.sql | 11 +++++
src/test/feature/catalog/test_guc.cpp | 28 +++++++++++
src/test/feature/lib/sql_util.cpp | 16 +++++--
src/test/feature/lib/sql_util.h | 5 ++
9 files changed, 218 insertions(+), 48 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c742cd71/src/backend/cdb/executormgr.c
----------------------------------------------------------------------
diff --git a/src/backend/cdb/executormgr.c b/src/backend/cdb/executormgr.c
index 4fecdea..e35f10b 100644
--- a/src/backend/cdb/executormgr.c
+++ b/src/backend/cdb/executormgr.c
@@ -864,29 +864,44 @@ addOneOption(PQExpBufferData *buffer, struct config_generic * guc)
{
struct config_string *sguc = (struct config_string *) guc;
const char *str = *sguc->variable;
- int j,
- start,
- end;
- char temp[1024];
+ unsigned int j, start, size;
+ char *temp, *new_temp;
- end = strlen(str);
+ size = 256;
+ temp = palloc(size + 8);
+ if (temp == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
j = 0;
- for (start = 0; start < end; ++start)
+ for (start = 0; start < strlen(str); ++start)
{
- if (str[start] == ' ')
- continue;
+ if (j == size)
+ {
+ size *= 2;
+ new_temp = repalloc(temp, size + 8);
+ if (new_temp == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+ temp = new_temp;
+ }
- if (str[start] == '"' || str[start] == '\'')
+ if (str[start] == ' ')
+ {
+ temp[j++] = '\\';
+ temp[j++] = '\\';
+ } else if (str[start] == '"' || str[start] == '\'')
temp[j++] = '\\';
- temp[j++] = str[start];
- if (j >= 1023)
- return false;
+ temp[j++] = str[start];
}
- temp[j] = '\0';
+ temp[j] = '\0';
appendPQExpBuffer(buffer, " -c %s=%s", guc->name, temp);
+ pfree(temp);
+
return true;
}
}
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c742cd71/src/backend/postmaster/postmaster.c
----------------------------------------------------------------------
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index b172eb4..cefcb86 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5555,34 +5555,61 @@ report_fork_failure_to_client(Port *port, int errnum)
/*
- * split_opts -- split a string of options and append it to an argv array
+ * pg_split_opts -- split a string of options and append it to an argv array
*
- * NB: the string is destructively modified!
+ * The caller is responsible for ensuring the argv array is large enough. The
+ * maximum possible number of arguments added by this routine is
+ * (strlen(optstr) + 1) / 2.
*
- * Since no current POSTGRES arguments require any quoting characters,
- * we can use the simple-minded tactic of assuming each set of space-
- * delimited characters is a separate argv element.
- *
- * If you don't like that, well, we *used* to pass the whole option string
- * as ONE argument to execl(), which was even less intelligent...
+ * Because some option values can contain spaces we allow escaping using
+ * backslashes, with \\ representing a literal backslash.
*/
static void
-split_opts(char **argv, int *argcp, char *s)
+pg_split_opts(char **argv, int *argcp, const char *optstr)
{
- while (s && *s)
+ StringInfoData s;
+
+ initStringInfo(&s);
+
+ while (*optstr)
{
- while (isspace((unsigned char) *s))
- ++s;
- if (*s == '\0')
+ bool last_was_escape = false;
+
+ resetStringInfo(&s);
+
+ /* skip over leading space */
+ while (isspace((unsigned char) *optstr))
+ optstr++;
+
+ if (*optstr == '\0')
break;
- argv[(*argcp)++] = s;
- while (*s && !isspace((unsigned char) *s))
- ++s;
- if (*s)
- *s++ = '\0';
+
+ /*
+ * Parse a single option, stopping at the first space, unless it's
+ * escaped.
+ */
+ while (*optstr)
+ {
+ if (isspace((unsigned char) *optstr) && !last_was_escape)
+ break;
+
+ if (!last_was_escape && *optstr == '\\')
+ last_was_escape = true;
+ else
+ {
+ last_was_escape = false;
+ appendStringInfoChar(&s, *optstr);
+ }
+
+ optstr++;
+ }
+
+ /* now store the option in the next argv[] position */
+ argv[(*argcp)++] = pstrdup(s.data);
}
-}
+ pfree(s.data);
+}
/*
* BackendInitialize -- initialize an interactive (postmaster-child)
@@ -5815,7 +5842,7 @@ BackendRun(Port *port)
*
* The maximum possible number of commandline arguments that could come
* from ExtraOptions or port->cmdline_options is (strlen + 1) / 2; see
- * split_opts().
+ * pg_split_opts().
* ----------------
*/
maxac = 10; /* for fixed args supplied below */
@@ -5831,10 +5858,9 @@ BackendRun(Port *port)
/*
* Pass any backend switches specified with -o in the postmaster's own
- * command line. We assume these are secure. (It's OK to mangle
- * ExtraOptions now, since we're safely inside a subprocess.)
+ * command line. We assume these are secure.
*/
- split_opts(av, &ac, ExtraOptions);
+ pg_split_opts(av, &ac, ExtraOptions);
/* Tell the backend what protocol the frontend is using. */
snprintf(protobuf, sizeof(protobuf), "-v%u", port->proto);
@@ -5848,11 +5874,10 @@ BackendRun(Port *port)
av[ac++] = port->database_name;
/*
- * Pass the (insecure) option switches from the connection request. (It's
- * OK to mangle port->cmdline_options now.)
+ * Pass the (insecure) option switches from the connection request.
*/
if (port->cmdline_options)
- split_opts(av, &ac, port->cmdline_options);
+ pg_split_opts(av, &ac, port->cmdline_options);
av[ac] = NULL;
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c742cd71/src/backend/utils/misc/guc.c
----------------------------------------------------------------------
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 4bd2aa5..7300fee 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -12202,6 +12202,7 @@ ProcessGUCArray(ArrayType *array, GucSource source)
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("could not parse setting for parameter \"%s\"", name)));
free(name);
+ pfree(s);
continue;
}
@@ -12216,12 +12217,49 @@ ProcessGUCArray(ArrayType *array, GucSource source)
* GPSQL needs to dispatch the database/user config to segments.
*/
if (Gp_role == GP_ROLE_DISPATCH)
- appendStringInfo(&MyProcPort->override_options, "-c %s=%s ", name, value);
- elog(DEBUG1, "gpsql guc: %s = %s", name , value);
+ {
+ unsigned int j, start, size;
+ char *temp, *new_temp;
+
+ size = 256;
+ temp = palloc(size + 8);
+ if (temp == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
+ j = 0;
+ for (start = 0; start < strlen(value); ++start)
+ {
+ if (j == size)
+ {
+ size *= 2;
+ new_temp = repalloc(temp, size + 8);
+ if (new_temp == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+ temp = new_temp;
+ }
+
+ if (value[start] == ' ')
+ {
+ temp[j++] = '\\';
+ temp[j++] = '\\';
+ } else if (value[start] == '"' || value[start] == '\'')
+ temp[j++] = '\\';
+
+ temp[j++] = value[start];
+ }
+
+ temp[j] = '\0';
+ appendStringInfo(&MyProcPort->override_options, "-c %s=%s ", name, temp);
+ elog(DEBUG1, "gpsql guc: %s = %s", name, temp);
+ pfree(temp);
+ }
free(name);
- if (value)
- free(value);
+ free(value);
pfree(s);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c742cd71/src/test/feature/.gitignore
----------------------------------------------------------------------
diff --git a/src/test/feature/.gitignore b/src/test/feature/.gitignore
index 1b02500..65fc84f 100644
--- a/src/test/feature/.gitignore
+++ b/src/test/feature/.gitignore
@@ -1,5 +1,21 @@
doc/
+# test binaries
+feature-test
+
# test generated files
**/*.out
**/*.diff
+
+ExternalSource/ans/external_oid.ans
+ExternalSource/ans/exttab1.ans
+ExternalSource/sql/external_oid.sql
+ExternalSource/sql/exttab1.sql
+UDF/ans/function_c.ans
+UDF/ans/function_creation.ans
+UDF/sql/function_c.sql
+UDF/sql/function_creation.sql
+testlib/ans/template.ans
+testlib/sql/template.sql
+utility/ans/copytest.csv
+utility/ans/onek.data
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c742cd71/src/test/feature/catalog/ans/guc.ans
----------------------------------------------------------------------
diff --git a/src/test/feature/catalog/ans/guc.ans b/src/test/feature/catalog/ans/guc.ans
new file mode 100644
index 0000000..7c4595c
--- /dev/null
+++ b/src/test/feature/catalog/ans/guc.ans
@@ -0,0 +1,26 @@
+CREATE TABLE DATE_TBL (f1 date);
+CREATE TABLE
+INSERT INTO DATE_TBL VALUES ('1957-04-09');
+INSERT 0 1
+SELECT f1 FROM DATE_TBL;
+ f1
+------------
+ 04/09/1957
+(1 row)
+
+SET DATESTYLE TO 'POSTGRES, MDY';
+SET
+SELECT f1 FROM DATE_TBL;
+ f1
+------------
+ 04-09-1957
+(1 row)
+
+SET DATESTYLE TO 'POSTGRES, DMY';
+SET
+SELECT f1 FROM DATE_TBL;
+ f1
+------------
+ 09-04-1957
+(1 row)
+
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c742cd71/src/test/feature/catalog/sql/guc.sql
----------------------------------------------------------------------
diff --git a/src/test/feature/catalog/sql/guc.sql b/src/test/feature/catalog/sql/guc.sql
new file mode 100644
index 0000000..cf16538
--- /dev/null
+++ b/src/test/feature/catalog/sql/guc.sql
@@ -0,0 +1,11 @@
+CREATE TABLE DATE_TBL (f1 date);
+INSERT INTO DATE_TBL VALUES ('1957-04-09');
+
+SELECT f1 FROM DATE_TBL;
+
+SET DATESTYLE TO 'POSTGRES, MDY';
+SELECT f1 FROM DATE_TBL;
+
+SET DATESTYLE TO 'POSTGRES, DMY';
+SELECT f1 FROM DATE_TBL;
+
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c742cd71/src/test/feature/catalog/test_guc.cpp
----------------------------------------------------------------------
diff --git a/src/test/feature/catalog/test_guc.cpp b/src/test/feature/catalog/test_guc.cpp
new file mode 100644
index 0000000..acf5476
--- /dev/null
+++ b/src/test/feature/catalog/test_guc.cpp
@@ -0,0 +1,28 @@
+#include "gtest/gtest.h"
+
+#include "lib/sql_util.h"
+#include "lib/file_replace.h"
+
+using std::string;
+using hawq::test::FileReplace;
+
+class TestGuc: public ::testing::Test
+{
+ public:
+ TestGuc() {};
+ ~TestGuc() {};
+};
+
+// Mainly test per-db guc via "alter database" which installcheck-good does not seem to cover.
+// Need to include other guc test cases (at least installcheck-good: guc.sql/guc.ans)
+TEST_F(TestGuc, per_db_guc_with_space)
+{
+ hawq::test::SQLUtility util(hawq::test::MODE_DATABASE);
+ string cmd;
+
+ cmd = "alter database " + util.getDbName() + " set datestyle to 'sql, mdy'";
+ util.execute(cmd);
+
+ util.execSQLFile("catalog/sql/guc.sql", "catalog/ans/guc.ans");
+}
+
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c742cd71/src/test/feature/lib/sql_util.cpp
----------------------------------------------------------------------
diff --git a/src/test/feature/lib/sql_util.cpp b/src/test/feature/lib/sql_util.cpp
index 37d8d49..f0568a2 100644
--- a/src/test/feature/lib/sql_util.cpp
+++ b/src/test/feature/lib/sql_util.cpp
@@ -35,17 +35,19 @@ SQLUtility::SQLUtility(SQLUtilityMode mode)
};
getConnection();
- if (MODE_SCHEMA == mode) {
+ if (mode == MODE_SCHEMA) {
schemaName = string(test_info->test_case_name()) + "_" + test_info->name();
+ databaseName = HAWQ_DB;
exec("DROP SCHEMA IF EXISTS " + schemaName + " CASCADE");
exec("CREATE SCHEMA " + schemaName);
-
+ sql_util_mode = MODE_SCHEMA;
} else {
schemaName = HAWQ_DEFAULT_SCHEMA;
databaseName = "db_" + string(test_info->test_case_name()) + "_" + test_info->name();
std::transform(databaseName.begin(), databaseName.end(), databaseName.begin(), ::tolower);
- exec("DROP DATABASE IF EXISTS " + databaseName);
+ exec("DROP DATABASE IF EXISTS " + databaseName);
exec("CREATE DATABASE " + databaseName);
+ sql_util_mode = MODE_DATABASE;
}
}
@@ -55,12 +57,16 @@ SQLUtility::~SQLUtility() {
exec("DROP SCHEMA " + schemaName + " CASCADE");
}
- if (!databaseName.empty()) {
+ if (sql_util_mode == MODE_DATABASE) {
exec("DROP DATABASE " + databaseName);
}
}
}
+std::string SQLUtility::getDbName() {
+ return databaseName;
+}
+
void SQLUtility::exec(const string &sql) {
EXPECT_EQ(0, (conn->runSQLCommand(sql)).getLastStatus())
<< conn->getLastResult();
@@ -193,7 +199,7 @@ const string SQLUtility::generateSQLFile(const string &sqlFile) {
}
out << "-- start_ignore" << std::endl;
out << "SET SEARCH_PATH=" + schemaName + ";" << std::endl;
- if (!databaseName.empty()) {
+ if (sql_util_mode == MODE_DATABASE) {
out << "\\c " << databaseName << std::endl;
}
out << "-- end_ignore" << std::endl;
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c742cd71/src/test/feature/lib/sql_util.h
----------------------------------------------------------------------
diff --git a/src/test/feature/lib/sql_util.h b/src/test/feature/lib/sql_util.h
index 8f8a6df..9bf1f90 100644
--- a/src/test/feature/lib/sql_util.h
+++ b/src/test/feature/lib/sql_util.h
@@ -34,6 +34,10 @@ class SQLUtility {
SQLUtility(SQLUtilityMode mode = MODE_SCHEMA);
~SQLUtility();
+ // Get the test database name
+ // @return string of the test database name
+ std::string getDbName();
+
// Execute sql command
// @param sql The given sql command
// @param check true(default) if expected correctly executing, false otherwise
@@ -104,6 +108,7 @@ class SQLUtility {
private:
std::string schemaName;
std::string databaseName;
+ SQLUtilityMode sql_util_mode;
std::unique_ptr<hawq::test::PSQL> conn;
std::string testRootPath;
const ::testing::TestInfo *const test_info;