Skip to content

Fix analyzedb failure on non-ASCII identifiers #1938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
tuhaihe wants to merge 1 commit into
base: main
Choose a base branch
from
+44 −20
Draft

Fix analyzedb failure on non-ASCII identifiers #1938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 23 additions & 20 deletions gpMgmt/bin/analyzedb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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'])
Expand Down Expand Up @@ -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])
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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])
Expand All @@ -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:
Expand All @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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

Expand All @@ -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])
Expand All @@ -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):
Expand Down
12 changes: 12 additions & 0 deletions gpMgmt/bin/gppylib/db/dbconn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions gpMgmt/test/behave/mgmt_utils/analyzedb.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading