Skip to content
Snippets Groups Projects
Unverified Commit 32a0d6cb authored by dpsutton's avatar dpsutton Committed by GitHub
Browse files

Effective type in result cols (#17533)

* Include the results base type as the cols effective type

previously was merged the effective type from the col which caused
issues with dates when selecting a particular temporal unit, ie month.

Queries like this return months as integers so the base and effective
types are :type/Integer, and something somewhere else is responsible
for the unit conversion of `1` to January (or feb, months get weird i
think)

```clojure

;; after
{:description "The date and time an order was submitted.",
 :unit :month-of-year,
 :name "CREATED_AT",
 :field_ref [:field 4 {:temporal-unit :month-of-year}],
 :effective_type :type/Integer, ;; we have the proper effective type
 :display_name "Created At",
 :base_type :type/Integer}

;; before:
{:description "The date and time an order was submitted.",
 :unit :month-of-year,
 :name "CREATED_AT",
 :field_ref [:field 4 {:temporal-unit :month-of-year}],
 :effective_type :type/DateTime, ;; included effective type from db
 :display_name "Created At",
 :base_type :type/Integer}
```

* tests

* Driver tests

* Ignore effective/base types in breakout tests

sparksql is weird and its totally unrelated to the file under test

* Use correct options for datetimes that are projected to month, etc

When aggregating by a datetime column, you can choose a unit: month,
hour-of-day, week, etc. You often (always?) end up with an
integer. Previously the base_type was (inexplicably) :type/Integer and
the effective_type was :type/DateTime which really just didn't make
sense. The ideal solution is :type/DateTime as the base_type and
:type/Integer as the effective_type. But this breaks the frontend as
it expects to treat these columns in a special way. But it expected
them to report as date columns.

I've changed it only here. I thought about making isDate understand
the `column.unit` attribute and recognize that these are in fact
dates, but that seems wrong. These are integers that are special
cases. It seems that the contexts that need to talk about dates should
understand integers in a special way than all date code needs to be
aware that integers might flow through.

This might mean extra work but ultimately feels better as the correct
solution.

* unit is not default
parent d469362e
No related branches found
No related tags found
No related merge requests found
Showing
with 78 additions and 19 deletions
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment