aminghadersohi opened a new pull request, #44685: URL: https://github.com/apache/superset/pull/44685
### SUMMARY `OracleEngineSpec` defines no `column_type_mappings`, so it relies on the base mappings in `BaseEngineSpec`. Those have no pattern for Oracle's own type names. `TableColumn.type_generic` calls `get_column_spec(...)`, which returns `None` for these types, so the dataset columns get no generic type: - `NUMBER`, `NUMBER(p, s)`, `BINARY_FLOAT` and `BINARY_DOUBLE`: not numeric. `NUMBER` is Oracle's primary numeric type. - `CLOB` and `NCLOB`: not strings. With `type_generic` null, a `NUMBER` column is not in `SqlaTable.num_cols` and `TableColumn.is_numeric` is `False`. In Explore, dragging it into Metrics gives no default `SUM` aggregate (`createAdhocMetricFromColumn`). Dropping a folder of columns into Metrics skips it (`isColumnSupportedForMetricAggregation`). Only columns whose reflected type string happens to be `INTEGER` or `DOUBLE PRECISION` were treated as numeric. This PR adds Oracle `column_type_mappings`: | Oracle type (as reflected) | SQLAlchemy type | Generic type | |---|---|---| | `NUMBER`, `NUMBER(p, s)` | `Numeric` | NUMERIC | | `BINARY_FLOAT`, `BINARY_DOUBLE` | `Float` | NUMERIC | | `CLOB`, `NCLOB` | `Text` | STRING | `BLOB` and `RAW` are binary and are intentionally left unmapped. Types already covered by the base mappings (`INTEGER`, `DOUBLE PRECISION`, `VARCHAR`, `DATE`, `TIMESTAMP`) are unchanged, and the new tests pin that. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A. The observed behaviour, running `create_app()`, then `Database`, `SqlaTable` and `fetch_metadata()` against a live Oracle Database Free 23 container, before this change: | Oracle DDL | `TableColumn.type` | `type_generic` | `is_numeric` | |---|---|---|---| | `NUMBER` | `NUMBER` | None | False | | `NUMBER(10,2)` | `NUMBER(10, 2)` | None | False | | `NUMBER(19,0)` | `NUMBER(19, 0)` | None | False | | `BINARY_DOUBLE` | `BINARY_DOUBLE` | None | False | | `INTEGER` | `INTEGER` | NUMERIC | True | | `FLOAT` | `DOUBLE PRECISION` | NUMERIC | True | | `CLOB` | `CLOB` | None | False (`is_string` False) | The dataset's `num_cols` contained only the `INTEGER` and `FLOAT` columns. ### TESTING INSTRUCTIONS ``` pytest tests/unit_tests/db_engine_specs/test_oracle.py ``` The new `test_get_column_spec` cases for `NUMBER`, `NUMBER(p, s)`, `BINARY_*` and `*CLOB` fail without the change (7 failures) and pass with it (31 passed). Base-mapped types are asserted unchanged, and `BLOB`/`RAW` are asserted to remain unmapped. To check manually: on an Oracle database, create a table with a `NUMBER(10,2)` column, add it as a dataset, and confirm the column is typed numeric. Dragging it into Metrics should now default to `SUM`. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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]
