aminghadersohi opened a new pull request, #44711:
URL: https://github.com/apache/superset/pull/44711
### SUMMARY
Fixes two defects in `DynamoDBEngineSpec`. Both block charts on a DynamoDB
virtual dataset.
1. **NUMBER has no generic type.** PyDynamoDB describes DynamoDB numbers
with the type code `NUMBER` (`pydynamodb.sql.common.DataTypes.NUMBER`). No
default column type mapping matches it, so `get_column_spec("NUMBER")` returns
`None`. A numeric column in a dataset therefore gets no `type_generic`. This
now maps to `Numeric` / `GenericDataType.NUMERIC`.
2. **Sorted charts fail.** The PyDynamoDB dialect omits top-level column
aliases, because PartiQL has none. A chart query therefore compiles as `SELECT
label, SUM(amount), COUNT(*) FROM (...) AS virtual_table GROUP BY label ORDER
BY total DESC`: the `AS total` label is dropped, but the `ORDER BY` still
references it. This fails with `no such column: total` (the superset connector
evaluates it in SQLite). With `allows_alias_in_orderby = False`, Superset
orders by the expression instead: `ORDER BY SUM(amount) DESC`.
Related: passren/PyDynamoDB#86 types `cursor.description` from the returned
values. With it, a virtual dataset over `SELECT id, amount, ts, label` gets
NUMBER and DATETIME columns. With the mapping in this PR, those columns become
NUMERIC and TEMPORAL.
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/db_engine_specs/test_dynamodb.py`: 10 passed. On
master, 3 of the new tests fail.
- End to end against DynamoDB Local 3.3.1 with PyDynamoDB 0.8.0
(`connector=superset`). The type fix from passren/PyDynamoDB#86 was applied,
and the requests went through the REST API. The flow was: SQL Lab `SELECT`,
then a virtual dataset (id/amount NUMERIC, ts TEMPORAL with `is_dttm`), then a
saved table chart with `SUM(amount)` and `COUNT(*)` by label, ordered by the
SUM metric, then a P1D time-grain query.
- Before this change, the chart step failed with `no such column: total`.
- After it, every step passed.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] 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]
