kaya-abdullah commented on code in PR #45309:
URL: https://github.com/apache/superset/pull/45309#discussion_r4288787170
##########
superset/sql/parse.py:
##########
@@ -1000,6 +1003,82 @@ def _comment_re_for(dialect: Dialects | None) ->
re.Pattern[str]:
)
+# Oracle 11g and older cap identifiers at 30 bytes.
+ORACLE_LEGACY_MAX_IDENTIFIER_LENGTH = 30
+
+
+def _oracle_output_identifier(projection: exp.Expression) -> exp.Identifier |
None:
+ """
+ Return the identifier naming a projected column, if it has one.
+ """
+ if isinstance(projection, exp.Alias):
+ identifier = projection.args.get("alias")
+ elif isinstance(projection, exp.Column):
+ identifier = projection.this
+ else:
+ return None
+ return identifier if isinstance(identifier, exp.Identifier) else None
+
+
+def _oracle_name_key(identifier: exp.Identifier) -> str:
+ """
+ Return the name Oracle compares, since unquoted names are case-insensitive.
+ """
+ return identifier.name if identifier.quoted else identifier.name.upper()
+
+
+def _unique_oracle_alias(identifier: exp.Identifier, taken: set[str]) ->
exp.Identifier:
+ """
+ Return a `_2`, `_3`, ... variant of the identifier that isn't taken yet.
+
+ The limit is in bytes, so a multibyte name is cut on a character boundary.
+ """
+ suffix = 2
+ while True:
+ tail = f"_{suffix}"
+ budget = ORACLE_LEGACY_MAX_IDENTIFIER_LENGTH - len(tail)
+ stem = identifier.name.encode()[:budget].decode(errors="ignore")
+ alias = exp.to_identifier(stem + tail, quoted=identifier.quoted)
+ if _oracle_name_key(alias) not in taken:
+ return alias
+ suffix += 1
+
+
+def _deduplicate_oracle_output_names(query: exp.Expression) -> None:
+ """
+ Rename repeated output columns so the query can be wrapped on Oracle.
+
+ Oracle rejects `SELECT * FROM (<query>)` when the query returns two columns
+ with the same name, as in `SELECT a.id, b.id FROM a JOIN b ...`, with
+ "ORA-00918: column ambiguously defined". The second and later copies get a
+ `_2`, `_3`, ... suffix instead. Star projections are left alone, since
their
Review Comment:
You're right, that framing came from how I ran into it and it hides why the
function is there. Reworded the docstring and the comment above the
`WRAP_ROWNUM` branch in b6051bdba8: the wrapper fails with ORA-00918 on 11g
too, and both shapes it leaves alone are written down now, qualified stars and
repeated expressions without an alias.
I ran your four queries on 11.2.0.2 and got the same results. `SELECT 1 + 1
AS x, 1 + 1 AS x` is the variant the rename does fix, it comes back as X and
X_2. Updated the PR description too.
##########
superset/db_engine_specs/oracle.py:
##########
@@ -51,14 +62,83 @@ class OracleEngineSpec(BaseEngineSpec):
"notes": (
"Previously used cx_Oracle, now uses oracledb. Legacy LONG columns
are "
"treated as text, not numeric data, so Explore does not select SUM
as "
- "their default aggregate."
+ "their default aggregate. On Oracle 11g and older, which reject
FETCH "
+ "FIRST, row limits use ROWNUM. The server version is detected on "
+ "first use, or can be set under Advanced > Other > Version."
),
"docs_url":
"https://python-oracledb.readthedocs.io/en/latest/user_guide/installation.html";,
}
force_column_alias_quotes = True
max_column_name_length = 128
Review Comment:
Agreed, that one needs the same version check as `limit_method`, so it fits
better in its own PR.
--
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]
