codeant-ai-for-open-source[bot] commented on code in PR #37019:
URL: https://github.com/apache/superset/pull/37019#discussion_r2677267235
##########
superset/db_engine_specs/redshift.py:
##########
@@ -103,6 +103,24 @@ class RedshiftEngineSpec(BasicParametersMixin,
PostgresBaseEngineSpec):
),
}
+ @classmethod
+ def normalize_table_name_for_upload(
+ cls,
+ table_name: str,
+ schema_name: str | None = None,
+ ) -> tuple[str, str | None]:
+ """
+ Redshift folds unquoted identifiers to lowercase.
+
+ :param table_name: The table name to normalize
+ :param schema_name: The schema name to normalize (optional)
+ :return: Tuple of (normalized_table_name, normalized_schema_name)
+ """
+ return (
+ table_name.lower(),
+ schema_name.lower() if schema_name else None,
+ )
Review Comment:
**Suggestion:** Logic bug: `normalize_table_name_for_upload` lowercases
every identifier unconditionally, which will incorrectly alter quoted
identifiers (which are case-sensitive and must be preserved) and may strip
meaningful case; detect quoted identifiers (surrounded by double quotes) and
preserve their case (strip the quotes but do not lowercase), otherwise fold
unquoted identifiers to lowercase. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
def _normalize(name: str | None) -> str | None:
if name is None:
return None
name = name.strip()
# Preserve quoted identifiers (case-sensitive) but strip
surrounding quotes.
if len(name) >= 2 and name[0] == '"' and name[-1] == '"':
return name[1:-1]
# Unquoted identifiers are folded to lowercase.
return name.lower()
return _normalize(table_name), _normalize(schema_name)
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The current implementation unconditionally lowercases both table and schema
names and will thus mangle quoted, case-sensitive identifiers. SQL engines
(including Postgres/Redshift) preserve case when identifiers are quoted. The
proposed improvement correctly preserves quoted identifiers (stripping quotes
but keeping case) and lowercases unquoted ones. This fixes a real semantic bug.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/redshift.py
**Line:** 119:122
**Comment:**
*Logic Error: Logic bug: `normalize_table_name_for_upload` lowercases
every identifier unconditionally, which will incorrectly alter quoted
identifiers (which are case-sensitive and must be preserved) and may strip
meaningful case; detect quoted identifiers (surrounded by double quotes) and
preserve their case (strip the quotes but do not lowercase), otherwise fold
unquoted identifiers to lowercase.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
