From ad4a8dcb7c5cfb6212ac0df5d60daa5117f4ab2a Mon Sep 17 00:00:00 2001 From: Dianjin Wang Date: Fri, 28 Aug 2026 18:04:31 +0800 Subject: [PATCH] Fix analyzedb failure on non-ASCII identifiers analyzedb aborted with UnicodeEncodeError whenever the database held a schema, table or column whose name is not pure ASCII: File "analyzedb", line 985, in regclass_schema_tbl return "to_regclass('%s')" % (pg.escape_string(schema_tbl)) UnicodeEncodeError: 'ascii' codec can't encode characters in position 13-19: ordinal not in range(128) The SQL literals were escaped with the module level pg.escape_string() from PyGreSQL. That function is not bound to a connection, so it has no client encoding to work with and always encodes str arguments as ASCII -- see pg_escape_string() in PyGreSQL's pgmodule.c, which passes pg_encoding_ascii. The connection method conn.escape_string() uses PQclientEncoding() and PQescapeStringConn() instead, and handles any encoding. This is still the case in the latest PyGreSQL, so upgrading the bundled version would not have helped. Add dbconn.escapeString(conn, value), which escapes through the connection, and use it for all five call sites in analyzedb. get_oid_str() and regclass_schema_tbl() now take the connection so that they can reach it; every one of their callers already had one at hand. Two nearby spots interpolated names into SQL without escaping them at all, and are now escaped the same way: the schema name given to "analyzedb -s ", and the schema and table name passed to GET_LEAF_PARTITIONS_SQL. The behave scenario meant to cover this ("analyzedb can handle the table name with special utf-8 characters") only created a TEMP table, and temp schemas are skipped further down in the flow, so it had stopped exercising the failing path. Extend it with a permanent table, a non-ASCII schema and the -s path, and assert that those tables really appear in the output rather than only checking the exit code. The same pattern is still present in gpload.py and in Escape() and escapeArrayElement() in gppylib/utils.py, which affect gpload, gpsd and minirepro. Those are left for a follow-up. Reported-by: vsbace Assisted-by: Claude Code See: Issue#1929 --- gpMgmt/bin/analyzedb | 43 ++++++++++--------- gpMgmt/bin/gppylib/db/dbconn.py | 12 ++++++ .../test/behave/mgmt_utils/analyzedb.feature | 9 ++++ 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/gpMgmt/bin/analyzedb b/gpMgmt/bin/analyzedb index 48d8e16872c..bfb10857c3e 100755 --- a/gpMgmt/bin/analyzedb +++ b/gpMgmt/bin/analyzedb @@ -172,7 +172,7 @@ def validate_schema_exists(pg_port, dbname, schema): try: dburl = dbconn.DbURL(port=pg_port, dbname=dbname) conn = dbconn.connect(dburl) - count = dbconn.querySingleton(conn, "select count(*) from pg_namespace where nspname='%s';" % pg.escape_string(schema)) + count = dbconn.querySingleton(conn, "select count(*) from pg_namespace where nspname='%s';" % dbconn.escapeString(conn, schema)) if count == 0: raise ExceptionNoStackTraceNeeded("Schema %s does not exist in database %s." % (schema, dbname)) finally: @@ -573,7 +573,8 @@ class AnalyzeDb(Operation): elif self.schema: # all tables in a schema validate_schema_exists(self.pg_port, self.dbname, self.schema) logger.debug("getting all tables in the schema...") - all_schema_tables = run_sql(self.conn, GET_ALL_DATA_TABLES_IN_SCHEMA_SQL % self.schema) + all_schema_tables = run_sql(self.conn, + GET_ALL_DATA_TABLES_IN_SCHEMA_SQL % dbconn.escapeString(self.conn, self.schema)) # convert table name from ['public','foo'] to 'public.foo' and populate col_dict as all columns requested for schema_table in all_schema_tables: col_dict[(schema_table[0], schema_table[1])] = set(['-1']) @@ -655,7 +656,7 @@ class AnalyzeDb(Operation): if self.config_file is not None or self.single_table is not None: valid_tables = set() if len(ret) > 0: - oid_str = get_oid_str(ret) + oid_str = get_oid_str(self.conn, ret) qresult = run_sql(self.conn, GET_VALID_DATA_TABLES_SQL % oid_str) for schema_tbl in qresult: tup = (schema_tbl[0], schema_tbl[1]) @@ -678,14 +679,14 @@ class AnalyzeDb(Operation): def _get_ao_state(self, input_tables_set): logger.debug("getting ao state...") - oid_str = get_oid_str(input_tables_set) + oid_str = get_oid_str(self.conn, input_tables_set) ao_partition_info = run_sql(self.conn, GET_REQUESTED_AO_DATA_TABLE_INFO_SQL % oid_str) return get_partition_state_tuples(self.pg_port, self.dbname, 'pg_aoseg', ao_partition_info) def _get_lastop_state(self, input_tables_set): # oid, action, subtype, timestamp logger.debug("getting last operation states...") - oid_str = get_oid_str(input_tables_set) + oid_str = get_oid_str(self.conn, input_tables_set) qresult = run_sql(self.conn, GET_REQUESTED_LAST_OP_INFO_SQL % oid_str) ret = [] for r in qresult: @@ -828,7 +829,8 @@ class AnalyzeDb(Operation): return s def _expand_partition_tables(self, schema, parent): - qresult = run_sql(self.conn, GET_LEAF_PARTITIONS_SQL % (schema, parent)) + qresult = run_sql(self.conn, GET_LEAF_PARTITIONS_SQL % (dbconn.escapeString(self.conn, schema), + dbconn.escapeString(self.conn, parent))) if len(qresult) == 0: return [(schema, parent)] else: @@ -854,7 +856,7 @@ class AnalyzeDb(Operation): # The leaf_root_dict keeps track of the mapping between a leaf partition and its root partition # for the use of refreshing root stats. leaf_root_dict = {} - oid_str = get_oid_str(candidates) + oid_str = get_oid_str(self.conn, candidates) qresult = run_sql(self.conn, GET_LEAF_ROOT_MAPPING_SQL % oid_str) for mapping in qresult: leaf_root_dict[(mapping[0], mapping[1])] = (mapping[2], mapping[3]) @@ -876,7 +878,8 @@ class AnalyzeDb(Operation): 2. The leaf partitions (if range partitioned, especially by date) will be ordered in descending order of the partition key, so that newer partitions can be analyzed first. """ - candidate_regclass_str = get_oid_str(itertools.chain(candidates, root_partition_col_dict.keys())) + candidate_regclass_str = get_oid_str(self.conn, + itertools.chain(candidates, root_partition_col_dict.keys())) qresult = run_sql(self.conn, ORDER_CANDIDATES_BY_OID_SQL % candidate_regclass_str) ordered_candidates = [] for schema_tbl in qresult: @@ -886,7 +889,7 @@ class AnalyzeDb(Operation): def _expand_columns(self, col_dict, schema_table): if '-1' in col_dict[schema_table]: - cols = run_sql(self.conn, GET_COLUMN_NAMES_SQL % get_oid_str([schema_table])) + cols = run_sql(self.conn, GET_COLUMN_NAMES_SQL % get_oid_str(self.conn, [schema_table])) return set([x[0] for x in cols]) else: return col_dict[schema_table] @@ -969,20 +972,20 @@ def generate_timestamp(): return timestamp.strftime("%Y%m%d%H%M%S") -# The argument is a list of (schema, table) tuples. The output is a string containing an +# table_list is a list of (schema, table) tuples. The output is a string containing an # SQL expression like: to_regclass('schema.table'), that can be embedded safely in an SQL string. # The escaping is a bit tricky here: the schema and table name need to be double-quoted, and the # whole string needs to be in single quotes. -def get_oid_str(table_list): - return ','.join(map((lambda x: regclass_schema_tbl(x[0], x[1])), table_list)) +def get_oid_str(conn, table_list): + return ','.join(map((lambda x: regclass_schema_tbl(conn, x[0], x[1])), table_list)) # Returns a string that uses to_regclass instead of ::regclass # to_regclass returns NULL instead of an error if the table does not exist -def regclass_schema_tbl(schema, tbl): +def regclass_schema_tbl(conn, schema, tbl): schema_tbl = "%s.%s" % (escape_identifier(schema), escape_identifier(tbl)) - return "to_regclass('%s')" % (pg.escape_string(schema_tbl)) + return "to_regclass('%s')" % (dbconn.escapeString(conn, schema_tbl)) # Escape double-quotes in a string, so that the resulting string is suitable for @@ -1002,7 +1005,7 @@ def escape_identifier(str): def get_heap_tables_set(conn, input_tables_set): logger.debug("getting heap tables...") - oid_str = get_oid_str(input_tables_set) + oid_str = get_oid_str(conn, input_tables_set) dirty_tables = set() qresult = run_sql(conn, GET_REQUESTED_NON_AO_TABLES_SQL % oid_str) for row in qresult: @@ -1250,7 +1253,7 @@ def validate_tables(conn, tablenames): while curr_batch < nbatches: batch = tablenames[curr_batch * batch_size:(curr_batch + 1) * batch_size] - oid_str = ','.join(map((lambda x: "('%s')" % pg.escape_string(x)), batch)) + oid_str = ','.join(map((lambda x: "('%s')" % dbconn.escapeString(conn, x)), batch)) if not oid_str: break @@ -1266,9 +1269,9 @@ def get_include_cols_from_exclude(conn, schema, table, exclude_cols): """ Given a list of excluded columns of a table, get the list of included columns """ - quoted_exclude_cols = ','.join(["'%s'" % pg.escape_string(x) for x in exclude_cols]) + quoted_exclude_cols = ','.join(["'%s'" % dbconn.escapeString(conn, x) for x in exclude_cols]) - oid_str = regclass_schema_tbl(schema, table) + oid_str = regclass_schema_tbl(conn, schema, table) cols = run_sql(conn, GET_INCLUDED_COLUMNS_FROM_EXCLUDE_SQL % (oid_str, quoted_exclude_cols)) return set([x[0] for x in cols]) @@ -1281,8 +1284,8 @@ def validate_columns(conn, schema, table, column_list): if len(column_list) == 0: return - sql = VALIDATE_COLUMN_NAMES_SQL % (regclass_schema_tbl(schema, table), - ','.join(["'%s'" % pg.escape_string(x) for x in column_list])) + sql = VALIDATE_COLUMN_NAMES_SQL % (regclass_schema_tbl(conn, schema, table), + ','.join(["'%s'" % dbconn.escapeString(conn, x) for x in column_list])) valid_col_count = dbconn.querySingleton(conn, sql) if int(valid_col_count) != len(column_list): diff --git a/gpMgmt/bin/gppylib/db/dbconn.py b/gpMgmt/bin/gppylib/db/dbconn.py index b85f802d02b..596e15ead3f 100644 --- a/gpMgmt/bin/gppylib/db/dbconn.py +++ b/gpMgmt/bin/gppylib/db/dbconn.py @@ -274,6 +274,18 @@ def connect(dburl, utility=False, verbose=False, return Connection(connection) +def escapeString(conn, value): + """ + Escape a string so that it can be embedded in a single quoted SQL literal. + + Always use this instead of the module level pg.escape_string() or + pgdb.escape_string(): those are not bound to a connection, so they have no + client encoding to work with and assume ASCII, which makes them raise + UnicodeEncodeError on any non-ASCII input. Escaping through the connection + uses its actual client encoding instead. + """ + return conn._cnx.escape_string(value) + def execSQL(conn, sql, autocommit=True): """ Execute a sql command that is NOT expected to return any rows and expects to commit diff --git a/gpMgmt/test/behave/mgmt_utils/analyzedb.feature b/gpMgmt/test/behave/mgmt_utils/analyzedb.feature index 5809c7745a9..fa2393fe56d 100644 --- a/gpMgmt/test/behave/mgmt_utils/analyzedb.feature +++ b/gpMgmt/test/behave/mgmt_utils/analyzedb.feature @@ -1779,8 +1779,17 @@ Feature: Incrementally analyze the database Given database "special_encoding_db" is dropped and recreated And the user connects to "special_encoding_db" with named connection "default" And the user executes "CREATE TEMP TABLE spiegelungssätze (c1 int) DISTRIBUTED BY (c1)" with named connection "default" + And the user executes "CREATE TABLE öffentlich (c1 int) DISTRIBUTED BY (c1)" with named connection "default" + And the user executes "CREATE SCHEMA schöma" with named connection "default" + And the user executes "CREATE TABLE schöma.таблица (c1 int) DISTRIBUTED BY (c1)" with named connection "default" When the user runs "analyzedb -a -d special_encoding_db" Then analyzedb should return a return code of 0 + And analyzedb should print "öffentlich" to stdout + And analyzedb should print "таблица" to stdout + When the user runs "analyzedb -a -d special_encoding_db -s schöma" + Then analyzedb should return a return code of 0 + And analyzedb should print "таблица" to stdout + And the user drops the named connection "default" Scenario: analyzedb finds materialized views Given a materialized view "public.mv_test_view" exists on table "pg_class"