fix(oracle): respect version-specific identifier limits by shubhamjrd4559-sudo · Pull Request #45801 · apache/superset

shubhamjrd4559-sudo · 2026-10-28T06:14:16Z

SUMMARY

Oracle versions before 12.2 support identifiers up to 30 bytes, while Oracle 12.2 and later support up to 128. Superset currently applies the 128-character engine-spec limit to every Oracle version, which can produce ORA-00972 when generated chart aliases exceed the legacy limit.

This change makes the maximum alias length database-aware, uses 30 for Oracle releases before 12.2 and preserves the 128 limit for newer releases, and passes the database context through label generation. Other engine specs keep their existing class-level limit.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable.

TESTING INSTRUCTIONS

Added focused Oracle tests for configured versions before and after 12.2, plus alias truncation behavior.
Run pytest tests/unit_tests/db_engine_specs/test_oracle.py -q.
Local pytest could not start because Flask is not installed in this environment; CI results will provide full test validation.

ADDITIONAL INFORMATION

Has associated issue: References fix(oracle): follow-ups from #45309 - label-length version check + nested-AND ROWNUM detection #45758 (Oracle identifier-length follow-up only; nested-AND ROWNUM handling is tracked separately in fix(sql): find a ROWNUM cap nested inside a parenthesized AND group #45771).
Required feature flags: None.
Changes UI
Includes DB Migration
Introduces new feature or API
Removes existing feature or API

codeant-ai-for-open-source · 2026-10-28T06:24:08Z

        if (
-            cls.max_column_name_length
-            and len(label_mutated) > cls.max_column_name_length
+            max_column_name_length
+            and len(label_mutated) > max_column_name_length
        ):


Suggestion: Oracle enforces this legacy limit in bytes, but len() counts Unicode characters; multibyte labels under 30 characters pass through and can still fail with ORA-00972.

Assessment: 🟠 Major · 🔁 Occurrence: Rarely · 🏷️ Logic error

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/db_engine_specs/base.py **Line:** 2719:2722 **Comment:** *Logic Error: Oracle enforces this legacy limit in bytes, but `len()` counts Unicode characters; multibyte labels under 30 characters pass through and can still fail with ORA-00972. 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. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

codeant-ai-for-open-source · 2026-10-28T06:24:08Z

+            and len(label_mutated) > max_column_name_length
        ):
-            label_mutated = cls._truncate_label(label)
+            label_mutated = cls._truncate_label(label, max_column_name_length)


Suggestion: BigQueryEngineSpec and DatastoreEngineSpec override _truncate_label with one parameter, so over-limit labels now raise TypeError when this call passes two.

Assessment: 🟠 Major · 🔁 Occurrence: Sometimes · 🏷️ Api mismatch

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/db_engine_specs/base.py **Line:** 2723:2723 **Comment:** *Api Mismatch: `BigQueryEngineSpec` and `DatastoreEngineSpec` override `_truncate_label` with one parameter, so over-limit labels now raise `TypeError` when this call passes two. 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. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of ef31e0d.

The truncation call now passes only label, matching the one-parameter overrides.

_{If that's not right, unresolve this thread and CodeAnt will leave it open.}

✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 83f9385.

The truncation call passes only label, matching the one-parameter _truncate_label overrides.

_{If that's not right, unresolve this thread and CodeAnt will leave it open.}

codeant-ai-for-open-source · 2026-10-28T06:24:39Z

CodeAnt PR Risk

🟢 Low Risk

The PR appears safe to merge; the Oracle byte-length limit is handled, and truncation still calls engine overrides with one argument.

Assessed commit: 83f93854076c

codecov · 2026-10-28T06:52:22Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.76%. Comparing base (a846d75) to head (83f9385).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #45801   +/-   ##
=======================================
  Coverage   84.75%   84.76%           
=======================================
  Files        3071     3071           
  Lines      206037   206064   +27     
  Branches    48506    48509    +3     
=======================================
+ Hits       174625   174668   +43     
+ Misses      28109    28098   -11     
+ Partials     3303     3298    -5

Flag	Coverage Δ
hive	`34.49% <32.50%> (+<0.01%)`	⬆️
mysql	`52.60% <47.50%> (-0.01%)`	⬇️
postgres	`52.61% <47.50%> (?)`
presto	`36.22% <42.50%> (+<0.01%)`	⬆️
python	`87.77% <100.00%> (+0.01%)`	⬆️
sqlite	`52.37% <47.50%> (-0.01%)`	⬇️
unit	`82.53% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

rebenitez1802

Request changes: aliases now fit Oracle's 30-byte limit before 12.2, and the SQL for 12.2+ and other engines is unchanged. But the PR has the same two regressions that were raised on #45675 (same #45758 item, opened earlier, and it already fixes both), and its unit tests fail once it's merged with current master. Details are inline.

rebenitez1802 · 2026-10-29T15:33:57Z

                        # Get the corresponding column from the subquery
                        subq_col_name = db_engine_spec.make_label_compatible(
-                            col_name + "__"
+                            col_name + "__", database=self.database
                        )
                        # Reference the column from the already-created aliased subquery
                        subq_col = subq_alias.c[subq_col_name]


🔴 High — "Group remaining as Others" crashes when Oracle hashes a series label

make_sqla_column_compatible sets sqla_col.key = label_expected, so subq_alias.c is keyed by col_name + "__", not by the hashed alias. With a series column of 29 bytes or more on Oracle 11.2, this lookup therefore raises KeyError: '7e0a3e56f6524e25280e79b853efa5' while building the query. Users get a 500, and "View query" breaks too. The same happens when a database's Version is set to 12c; on a 12.2+ server set that way, this query works on master and crashes on this branch. Looking the column up by its key fixes it, and the 30-byte aliases stay intact:

Suggested change

# Get the corresponding column from the subquery

subq_col_name = db_engine_spec.make_label_compatible(

col_name + "__"

col_name + "__", database=self.database

)

# Reference the column from the already-created aliased subquery

subq_col = subq_alias.c[subq_col_name]

# The subquery keys its columns by the expected label, which

# differs from the SQL alias when the engine shortened it

subq_col = subq_alias.c[col_name + "__"]

rebenitez1802 · 2026-10-29T15:33:57Z

+    @classmethod
+    def get_max_column_name_length(cls, database: Database) -> int | None:
+        """Use Oracle's 30-byte identifier limit before release 12.2."""
+        server_version = cls.get_server_version(database)


🔴 High — A failed version lookup is retried once per label

make_label_compatible now asks for the server version for every label (get_max_column_name_length → get_server_version), and a failed lookup isn't remembered. With no Version set and the database unreachable, building one chart query tries to connect 17–39 times, logging a traceback each time; master tries none while building the query. python-oracledb's default connect timeout is 20 s, so one chart can hang for about 340 s, well past the web server timeout. Fix: remember the failure for the current request (for example a set on flask.g, as #45675 does), so a chart makes at most one attempt and the next request still retries.

rebenitez1802 · 2026-10-29T15:33:57Z

        db_engine_spec = self.db_engine_spec
        series_column_labels = [
-            db_engine_spec.make_label_compatible(column)
+            db_engine_spec.make_label_compatible(column, database=self.database)


🔴 High — Unit tests fail once merged with master

#45681 landed after this PR's CI run. It added test_cache_key_collects_series_metric_for_normalized_labels, which mocks make_label_compatible with side_effect=lambda name: .... Merged with current master, both of its parametrizations fail with TypeError: <lambda>() got an unexpected keyword argument 'database'; they pass on master. The branch merges without conflicts and the CI here is green, so this would only show up on master. Fix: merge master and change the mock in tests/unit_tests/connectors/sqla/virtual_dataset_connections_test.py to lambda name, database=None: ....

shubhamjrd4559-sudo · 2026-10-31T05:31:41Z

Hi @rebenitez1802, thanks for the detailed review. I rebased this PR on the latest master and addressed the reported issues: the shortened series alias lookup, repeated Oracle version lookup failures within a request, and the test mock’s database argument. I’ve added regression tests, and CI is running on the updated commit. I also noticed #45675 covers the same issue—would you prefer I keep this PR open for re-review or close it in favor of #45675?

pull-request-size Bot added the size/L label Oct 28, 2026

codeant-ai-for-open-source Bot reviewed Oct 28, 2026

View reviewed changes

sadpandajoe requested review from gabotorresruiz and rebenitez1802 October 28, 2026 16:28

rebenitez1802 requested changes Oct 29, 2026

View reviewed changes

shubhamjrd4559-sudo added 4 commits October 31, 2026 10:53

fix(oracle): respect version-specific identifier limits

3475115

style: apply Ruff formatting

567a764

fix(oracle): handle byte limits and label overrides

fa73d44

fix(oracle): address review feedback

83f9385

shubhamjrd4559-sudo force-pushed the fix/oracle-label-length-version-check branch from ef31e0d to 83f9385 Compare October 31, 2026 05:27

Navigation Menu

Repository navigation

fix(oracle): respect version-specific identifier limits - #45801

fix(oracle): respect version-specific identifier limits#45801
shubhamjrd4559-sudo wants to merge 4 commits into
apache:masterfrom
shubhamjrd4559-sudo:fix/oracle-label-length-version-check

shubhamjrd4559-sudo commented Oct 28, 2026

Uh oh!

codeant-ai-for-open-source Bot Oct 28, 2026

Uh oh!

codeant-ai-for-open-source Bot Oct 28, 2026

Uh oh!

codeant-ai-for-open-source Bot Oct 28, 2026

Uh oh!

codeant-ai-for-open-source Bot Oct 31, 2026

Uh oh!

codeant-ai-for-open-source Bot commented Oct 28, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Oct 28, 2026 •

edited

Loading

Uh oh!

rebenitez1802 left a comment

Uh oh!

rebenitez1802 Oct 29, 2026

Uh oh!

rebenitez1802 Oct 29, 2026

Uh oh!

rebenitez1802 Oct 29, 2026

Uh oh!

shubhamjrd4559-sudo commented Oct 31, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

shubhamjrd4559-sudo commented Oct 28, 2026

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

codeant-ai-for-open-source Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodeAnt PR Risk

Uh oh!

codecov Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rebenitez1802 left a comment

Choose a reason for hiding this comment

Uh oh!

rebenitez1802 Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

rebenitez1802 Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

rebenitez1802 Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

shubhamjrd4559-sudo commented Oct 31, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codeant-ai-for-open-source Bot commented Oct 28, 2026 •

edited

Loading

codecov Bot commented Oct 28, 2026 •

edited

Loading