codeant-ai-for-open-source[bot] commented on code in PR #45309:
URL: https://github.com/apache/superset/pull/45309#discussion_r4277458327
##########
superset/sql/parse.py:
##########
@@ -1000,6 +1003,79 @@ 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.
+ """
+ suffix = 2
+ while True:
+ tail = f"_{suffix}"
+ stem = identifier.name[: ORACLE_LEGACY_MAX_IDENTIFIER_LENGTH -
len(tail)]
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `e846a23`.
The alias stem is now truncated by encoded byte length, then decoded on a
character boundary; the ASCII suffix is included in the byte budget.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset/sql/parse.py:
##########
@@ -2116,7 +2192,53 @@ def get_limit_value(self) -> int | None:
if isinstance(literal, exp.Literal) and literal.is_int:
return int(literal.name)
- return None
+ return self._get_rownum_limit()
+
+ def _get_rownum_limit(self) -> int | None:
+ """
+ Return the row cap of a top-level Oracle `ROWNUM` filter, if any.
+
+ Oracle releases before 12c have no `FETCH FIRST`, so a `ROWNUM <= n`
+ condition in the outer `WHERE` is how a query caps its rows there. It
is
+ also the shape `set_limit_value()` produces with `WRAP_ROWNUM`. Only
+ conditions that are `AND`-ed into the filter count: under an `OR` the
+ comparison no longer bounds the result.
+ """
+ if self._dialect != Dialects.ORACLE or not isinstance(self._parsed,
exp.Select):
+ return None
+
+ where = self._parsed.args.get("where")
+ if where is None:
+ return None
+
+ condition = where.this.unnest()
+ conditions = (
+ condition.flatten() if isinstance(condition, exp.And) else
[condition]
+ )
+ caps = []
+ for predicate in conditions:
+ predicate = predicate.unnest()
+ if isinstance(predicate, (exp.LTE, exp.LT)):
+ rownum, bound = predicate.this, predicate.expression
+ elif isinstance(predicate, (exp.GTE, exp.GT)):
+ bound, rownum = predicate.this, predicate.expression
+ else:
+ continue
+
+ rownum = rownum.unnest()
+ bound = bound.unnest()
+ if (
+ isinstance(rownum, exp.Column)
+ and not rownum.table
+ and rownum.name.upper() == "ROWNUM"
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `e846a23`.
The ROWNUM check now excludes quoted identifiers, so a quoted column named
`rownum` is not treated as the pseudocolumn.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
--
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]
