Skip to content

fix(oracle-driver): Build in-database pre-aggregations - #12601

Open
waralexrom wants to merge 6 commits into
masterfrom
oracle-preagg-ctas-as
Open

waralexrom wants to merge 6 commits into
masterfrom
oracle-preagg-ctas-as

Conversation

@waralexrom

Copy link
Copy Markdown
Member

What was broken

A rollup with external: false could not be built on Oracle:

  • BaseQuery#preAggregationLoadSql rendered the CTAS keyword from asSyntaxTable. OracleQuery sets that to '' because Oracle forbids AS before table aliases, so the load SQL came out as CREATE TABLE <name> SELECT ... and Oracle rejected it with ORA-00922: missing or invalid option.
  • Before the CTAS even ran, the build failed in createSchemaIfNotExists and getTablesQuery, which read information_schema.schemata / information_schema.tables. Oracle has neither (ORA-00942).

The driver is readOnly(), so external rollups go through Cube Store and never hit this path; only in-database rollups did.

Fix

  • preAggregationLoadSql always emits AS: in CTAS it is part of the statement, not an alias keyword. Every other dialect already rendered AS here, so their SQL is unchanged. The same code serves Tesseract and the legacy planner.
  • OracleDriver reads its own catalog: createSchemaIfNotExists checks all_users and throws a clear error when the schema is missing (an Oracle schema is a user account, which a build can't create); getTablesQuery reads all_tables and lowercases names so they match the unquoted names Cube generates.

Tests

New must build an in-database pre-aggregation case in the shared testQueries suite, enabled for oracle-full through a new includeInDatabasePreAggregationSuite option. It builds ECommerce.SAInternal in Oracle, reads the built table back with the raw driver and checks its rows against the same query answered through the API.

Pre-aggregation builds drop every table in their schema that they don't recognise, so the Oracle fixture now gets a dedicated test_preaggs schema from an init script, with CUBEJS_PRE_AGGREGATIONS_SCHEMA pointing at it.

Run locally against gvenzl/oracle-free:23-slim-faststart (yarn oracle-full --mode=local):

  • full suite passes with Tesseract and with the legacy planner;
  • with the CTAS change reverted, the new case fails with ORA-00922.

Fixes #3127

🤖 Generated with Claude Code

The pre-aggregation CTAS reused the table-alias keyword, which Oracle
leaves empty, so it was rendered without AS and Oracle rejected it.
Before that, the build failed on information_schema catalog lookups that
Oracle doesn't have; the driver now reads its own catalog for them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@waralexrom
waralexrom requested review from a team as code owners November 3, 2026 23:43
@github-actions github-actions Bot added driver:oracle Issues relating to the Oracle driver javascript Pull requests that update Javascript code data source driver labels Nov 3, 2026
@claude

claude Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @waralexrom's task in 1m 46s —— View job


Review: the fix is correct; 2 high findings (data-loss risk in the new error message, missing Oracle docs).

Details

Checklist

  • Read diff
  • Read changed files and surrounding code
  • Check review threads
  • Post review

Findings

Severity Location Issue
High packages/cubejs-oracle-driver/driver/OracleDriver.js:199 The error tells users to point CUBEJS_PRE_AGGREGATIONS_SCHEMA at an existing schema. dropOrphanedTables (PreAggregationLoader.ts:1141) drops every unrecognised table in that schema, so pointing it at the app schema deletes user tables. Suggested wording is on the inline comment.
High packages/cubejs-oracle-driver/driver/OracleDriver.js:195 oracle.mdx doesn't document the setup in-database pre-aggregations now need: a pre-created dedicated schema, CREATE/DROP/SELECT ANY TABLE grants, and nothing but pre-aggregations in that schema. Adding the docs, or a top-level author comment explaining why none are needed, resolves it.

Verified, no issues

  • BaseQuery.preAggregationLoadSql: hardcoding AS gives the same output on every dialect except Oracle. Only OracleQuery overrides asSyntaxTable. ClickHouse, BigQuery, MSSQL and KSQL have their own overrides and are unaffected. Tesseract and the legacy planner both reach this method through PreAggregations.ts:496.
  • getTablesQuery with LOWER(table_name) matches the ${schema}.${name} keys in PreAggregationLoader.ts:740,1142 and PreAggregations.ts:560, because Cube creates and drops these tables with unquoted names that Oracle stores uppercase.
  • Binds: ? becomes :cb_param_0 through normalizeParams. UPPER(?) won't match a schema created with a quoted mixed-case name, but that's an acceptable edge case.
  • Test: it builds SAInternal and checks the rollup rows against the API result, which is a solid end-to-end check. The broad ANY TABLE grants in oracle-init.sql are confined to the test fixture.
  • Comments in the diff are all 1–2 lines and each explains something the code doesn't.

Review threads

The listing returned 0 prior threads from me, so nothing was resolved or re-affirmed.

@codecov

codecov Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.18%. Comparing base (fe1e1cf) to head (adb09f6).
⚠️ Report is 5 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (fe1e1cf) and HEAD (adb09f6). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (fe1e1cf) HEAD (adb09f6)
cubesql 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #12601       +/-   ##
===========================================
- Coverage   83.16%   66.18%   -16.98%     
===========================================
  Files         531      258      -273     
  Lines      124731    21978   -102753     
  Branches     5290     5290               
===========================================
- Hits       103730    14546    -89184     
+ Misses      20206     6636    -13570     
- Partials      795      796        +1     
Flag Coverage Δ
cube-backend 66.18% <100.00%> (+0.03%) ⬆️
cubesql ?

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.

waralexrom and others added 2 commits November 4, 2026 10:50
The missing-schema error pointed users at any existing schema, while
builds drop every unrecognised table in it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@waralexrom
waralexrom requested a review from keydunov as a code owner November 4, 2026 09:51
@mintlify

mintlify Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
cubed3 🟢 Ready View Preview Nov 4, 2026, 11:08 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@github-actions github-actions Bot added the docs Issues that require a documentation improvement label Nov 4, 2026
@claude

claude Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @waralexrom's task in 2m 6s —— View job


Looks good to merge: 0 high, 0 medium, 2 low. Both prior threads are resolved.

Review details

Findings (inline)

  • Low: packages/cubejs-oracle-driver/driver/OracleDriver.js:206. LOWER(table_name) turns a quoted, mixed-case foreign table in the pre-aggregation schema into a name that the unquoted DROP TABLE in dropOrphanedTables can't find. That gives ORA-00942, so the build fails, and every later build fails too.
  • Low: docs-mintlify/admin/connect-to-data/data-sources/oracle.mdx:62. The ANY TABLE grants apply to the whole database, and the docs don't say so. On Oracle 23ai, GRANT ... ON SCHEMA cube_preaggs limits them to that schema.

What I checked

  • CTAS fix (BaseQuery.ts:4668): only OracleQuery overrides asSyntaxTable. BigQuery, MSSQL and ClickHouse have their own preAggregationLoadSql. So hardcoding AS changes only Oracle's output. This is shared code, so it fixes both Tesseract and the legacy planner. It also fixes the symptom in Preaggregation in oracle database fails #3127 ("forgotten 'as' clause").
  • Catalog overrides: createSchemaIfNotExists and getTablesQuery only run against the source driver for external: false rollups. External rollups call them on Cube Store (PreAggregationLoader.ts:269-274), so the existing Oracle → Cube Store path is unaffected. Both queries use bind parameters, so there's no injection risk from the schema name.
  • Test: it builds SAInternal in Oracle and checks the rows against the API result. Moving pre-aggregations into the dedicated test_preaggs schema keeps the orphan drop away from the fixture tables. oracle is in the drivers-tests CI matrix, so the new case will run there.
  • Docs: the new "Pre-aggregations" section in oracle.mdx covers the user-facing change: schema setup, CUBEJS_PRE_AGGREGATIONS_SCHEMA, and the warning about orphaned tables being dropped. The docs requirement is met.
  • Comments: all new comments are 3 lines or fewer, and each explains a reason the code can't express.

Review threads

The listing returned 2 of my own threads. I resolved both and re-affirmed none:

  • the request to add the drop warning to the createSchemaIfNotExists error, which is now in the message;
  • the docs request, which is covered by the new oracle.mdx section.

waralexrom and others added 2 commits November 4, 2026 11:08
… schema

A quoted mixed-case table came back lowercased, and the unquoted orphan
drop could not find it, failing every build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @waralexrom's task in 1m 36s —— View job


Looks good apart from one gap: the documented Oracle grant is missing CREATE ANY INDEX (1 medium, 0 high, 0 low).

Review details
  • Gather context (diff, changed files, linked issue)
  • Review driver, BaseQuery, tests, docs
  • Reconcile prior review threads
  • Post findings

Findings

  • Medium: docs-mintlify/admin/connect-to-data/data-sources/oracle.mdx:64 — the documented grant omits CREATE ANY INDEX. After the CTAS step, in-database rollups that declare indexes run CREATE INDEX cube_preaggs.<idx> ON cube_preaggs.<table>, and Oracle rejects that with ORA-01031. It applies to both planners. The new test doesn't catch it because SAInternal has no indexes. (inline)

Checked, no issue

  • BaseQuery.ts:4668: emitting AS unconditionally only changes Oracle's output, since OracleQuery is the only dialect that overrides asSyntaxTable. ClickHouse, MSSQL and BigQuery override preAggregationLoadSql and are untouched. The same path serves Tesseract and the legacy planner.
  • OracleDriver.js:205: LOWER(table_name) plus the table_name = UPPER(table_name) guard keeps the orphan-drop comparison at PreAggregationLoader.ts:740 consistent with the lowercase unquoted names Cube generates, and hides mixed-case tables from the drop.
  • OracleDriver.js:195: binding through normalizeParams works. The error message is clear and states the data-loss risk.
  • Docs: the new user-facing behaviour (Oracle in-database pre-aggregations need a pre-created schema) is documented on the Oracle data source page, including the warning that builds own the schema.

Review threads

The listing returned 2 of my prior threads. I resolved both, because the current diff addresses them: the LOWER(table_name) orphan-drop concern is fixed by the uppercase guard, and the database-wide grant warning is fixed by the schema-scoped 23ai grant in the docs. I re-affirmed none. I dropped one candidate finding, that the fixture's database-wide grant doesn't exercise the documented schema-scoped one, because an earlier round had explicitly accepted the broad grant in the fixture.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @waralexrom's task in 1m 40s —— View job


No issues found — 0 high, 0 medium, 0 low. 1 prior thread resolved.

Review details

What I checked

  • CTAS keyword (packages/cubejs-schema-compiler/src/adapter/BaseQuery.ts:4668): only OracleQuery sets asSyntaxTable to something other than AS. ClickHouse, MSSQL and BigQuery override preAggregationLoadSql themselves, so no other dialect's output changes. Tesseract and the legacy planner both use this method (Tesseract has no CTAS of its own), so the fix applies to both.
  • Catalog overrides (packages/cubejs-oracle-driver/driver/OracleDriver.js:194-210): ? binds go through normalizeParams. LOWER(table_name) "table_name" matches what PreAggregationLoadCache reads. The table_name = UPPER(table_name) filter keeps quoted mixed-case tables out of the drop list. dropTable uses unquoted names, so dropping the lowercased names reaches the same uppercase tables. The tableColumnTypes / information_schema.columns callers only run on external or lambda paths, so in-database builds don't hit them.
  • Linked issue Preaggregation in oracle database fails #3127: the reporter expected Cube to create dev_pre_aggregations. In Oracle a schema is a user account, so a build can't create one. The PR throws a clear error instead and documents the setup, which covers the reported symptom.
  • Tests: the new case reads the built table back with the raw driver and compares it with API results. The SA rollup has no indexes, so the fixture grant without CREATE ANY INDEX is enough. drivers-tests.yml runs oracle with both planners.
  • Docs: a new "Pre-aggregations" section in oracle.mdx covers the user-visible change. It includes the 23ai schema-scoped grant, the database-wide fallback for older releases, and a warning that builds drop unknown tables.
  • Comments: all are within 3 lines and explain why the code is the way it is.

Review threads

The listing returned 1 of my own threads. I resolved it (oracle.mdx, grants were missing CREATE ANY INDEX), because the docs now grant it. No threads re-affirmed.

Not done

I couldn't check the CI status of this head because gh pr checks needs approval in this run.

This branch was successfully deployed

1 active deployment
staging - docs-mintlify — adb09f6f Deployed Nov 4, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data source driver docs Issues that require a documentation improvement driver:oracle Issues relating to the Oracle driver javascript Pull requests that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preaggregation in oracle database fails

1 participant