From 5a80e561019b256674959d629afb009517f835c4 Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Thu, 29 Sep 2022 10:07:05 +0700
Subject: [PATCH] Advanced datetime extraction (#25277)
* Implement advanced date/time/zone manipulation, part 1
Incorporate new functions into MBQL and add tests:
- get-year
- get-quarter
- get-month
- get-day
- get-day-of-week
- get-hour
- get-minute
- get-second
* Fix BigQuery implementations to call extract
Mark as not supported in legacy driver
* Add date extraction fns for Postgres
* Disable in MongoDB (for now at least)
Disable in BigQuery (legacy driver)
Add implementations for presto-jdbc
* Misc cleanup from Jeff's PR
* Update Jeff's implementation of bigquery-cloud-sqk
* Reorganized tests
* Mongo
* Oracle
* Sqlserver
* Sqlite
* Add casting supports for presto
* Remove Jeff's implementation of presto-jdbc because its parent is
sql-jdbc
* Update presto-jdbc tests to use the same catalog for all datasets
* Add date extraction functions to the expression editor (#25382)
* make sure the semantic type of aggregated columns are integer
* no recursive call in annotate for date-extract func
* get-unit -> temporal-extract(column, unit)
* desguar nested datetime extraction too
---
.../run-presto-kerberos-integration-test.sh | 1 +
.../src/metabase/lib/expressions/config.js | 49 ++++++
.../lib/expressions/helper-text-strings.ts | 96 ++++++++++++
.../custom-column/cc-data-type.cy.spec.js | 33 ++++
.../bigquery_cloud_sdk/query_processor.clj | 26 ++--
.../mongo/src/metabase/driver/mongo.clj | 16 +-
.../metabase/driver/mongo/query_processor.clj | 52 ++++---
.../oracle/src/metabase/driver/oracle.clj | 27 ++--
.../test/metabase/driver/presto_jdbc_test.clj | 22 +--
.../test/metabase/test/data/presto_jdbc.clj | 16 +-
.../presto/src/metabase/driver/presto.clj | 10 ++
.../sqlite/src/metabase/driver/sqlite.clj | 8 +
.../src/metabase/driver/sqlserver.clj | 8 +
shared/src/metabase/mbql/normalize.cljc | 4 +
shared/src/metabase/mbql/predicates.cljc | 4 +
shared/src/metabase/mbql/schema.cljc | 100 ++++++++++--
shared/src/metabase/mbql/util.cljc | 24 ++-
shared/test/metabase/mbql/util_test.cljc | 17 ++-
src/metabase/driver.clj | 5 +
src/metabase/driver/postgres.clj | 28 ++--
src/metabase/driver/sql/query_processor.clj | 26 +++-
.../query_processor/middleware/desugar.clj | 9 +-
src/metabase/util/date_2.clj | 4 +-
src/metabase/util/honeysql_extensions.clj | 5 +-
.../automagic_dashboards/core_test.clj | 2 +-
.../middleware/desugar_test.clj | 8 +-
.../date_time_zone_functions_test.clj | 144 ++++++++++++++++++
27 files changed, 639 insertions(+), 105 deletions(-)
create mode 100644 test/metabase/query_processor_test/date_time_zone_functions_test.clj
diff --git a/.github/scripts/run-presto-kerberos-integration-test.sh b/.github/scripts/run-presto-kerberos-integration-test.sh
index 093841be8c5..e1f385a7a74 100755
--- a/.github/scripts/run-presto-kerberos-integration-test.sh
+++ b/.github/scripts/run-presto-kerberos-integration-test.sh
@@ -3,6 +3,7 @@
set -eo pipefail
# Need Java commands on $PATH, which apparently is not yet the case
+export JAVA_HOME="/usr/lib/jvm/msopenjdk-current"
export PATH="$PATH:$JAVA_HOME/bin"
# ensure java commmand is available
diff --git a/frontend/src/metabase/lib/expressions/config.js b/frontend/src/metabase/lib/expressions/config.js
index 2acf81fd527..a952ecac3af 100644
--- a/frontend/src/metabase/lib/expressions/config.js
+++ b/frontend/src/metabase/lib/expressions/config.js
@@ -321,6 +321,46 @@ export const MBQL_CLAUSES = {
type: "boolean",
args: ["expression", "expression"],
},
+ "get-year": {
+ displayName: `year`,
+ type: "number",
+ args: ["expression"],
+ },
+ "get-quarter": {
+ displayName: `quarter`,
+ type: "number",
+ args: ["expression"],
+ },
+ "get-month": {
+ displayName: `month`,
+ type: "number",
+ args: ["expression"],
+ },
+ "get-day": {
+ displayName: `day`,
+ type: "number",
+ args: ["expression"],
+ },
+ "get-day-of-week": {
+ displayName: `weekday`,
+ type: "number",
+ args: ["expression"],
+ },
+ "get-hour": {
+ displayName: `hour`,
+ type: "number",
+ args: ["expression"],
+ },
+ "get-minute": {
+ displayName: `minute`,
+ type: "number",
+ args: ["expression"],
+ },
+ "get-second": {
+ displayName: `second`,
+ type: "number",
+ args: ["expression"],
+ },
};
for (const [name, clause] of Object.entries(MBQL_CLAUSES)) {
@@ -392,6 +432,15 @@ export const EXPRESSION_FUNCTIONS = new Set([
"power",
"log",
"exp",
+ // date/time
+ "get-year",
+ "get-quarter",
+ "get-month",
+ "get-day",
+ "get-day-of-week",
+ "get-hour",
+ "get-minute",
+ "get-second",
// boolean
"contains",
"ends-with",
diff --git a/frontend/src/metabase/lib/expressions/helper-text-strings.ts b/frontend/src/metabase/lib/expressions/helper-text-strings.ts
index a702af8a7e9..3580e154594 100644
--- a/frontend/src/metabase/lib/expressions/helper-text-strings.ts
+++ b/frontend/src/metabase/lib/expressions/helper-text-strings.ts
@@ -571,6 +571,102 @@ const helperTextStrings: HelpText[] = [
],
hasDocsPage: true,
},
+ {
+ name: "get-year",
+ structure: "year(" + t`column` + ")",
+ description: t`Takes a datetime and returns an integer with the number of the year.`,
+ example: "year([" + t`Created At` + "])",
+ args: [
+ {
+ name: t`column`,
+ description: t`The datetime column.`,
+ },
+ ],
+ },
+ {
+ name: "get-quarter",
+ structure: "quarter(" + t`column` + ")",
+ description: t`Takes a datetime and returns an integer (1-4) with the number of the quarter in the year.`,
+ example: "quarter([" + t`Created At` + "])",
+ args: [
+ {
+ name: t`column`,
+ description: t`The datetime column.`,
+ },
+ ],
+ },
+ {
+ name: "get-month",
+ structure: "month(" + t`column` + ")",
+ description: t`Takes a datetime and returns an integer (1-12) with the number of the month in the year.`,
+ example: "month([" + t`Created At` + "])",
+ args: [
+ {
+ name: t`column`,
+ description: t`The datetime column.`,
+ },
+ ],
+ },
+ {
+ name: "get-day",
+ structure: "day(" + t`column` + ")",
+ description: t`Takes a datetime and returns an integer (1-31) with the number of the day of the month.`,
+ example: "day([" + t`Created At` + "])",
+ args: [
+ {
+ name: t`column`,
+ description: t`The datetime column.`,
+ },
+ ],
+ },
+ {
+ name: "get-day-of-week",
+ structure: "weekday(" + t`column` + ")",
+ description: t`Takes a datetime and returns an integer (1-7) with the number of the day of the week.`,
+ example: "weekday([" + t`Created At` + "])",
+ args: [
+ {
+ name: t`column`,
+ description: t`The datetime column.`,
+ },
+ ],
+ },
+ {
+ name: "get-hour",
+ structure: "hour(" + t`column` + ")",
+ description: t`Takes a datetime and returns an integer (0-23) with the number of the hour. No AM/PM.`,
+ example: "hour([" + t`Created At` + "])",
+ args: [
+ {
+ name: t`column`,
+ description: t`The datetime column.`,
+ },
+ ],
+ },
+ {
+ name: "get-minute",
+ structure: "minute(" + t`column` + ")",
+ description: t`Takes a datetime and returns an integer (0-59) with the number of the minute in the hour.`,
+ example: "minute([" + t`Created At` + "])",
+ args: [
+ {
+ name: t`column`,
+ description: t`The datetime column.`,
+ },
+ ],
+ },
+ {
+ name: "get-second",
+ structure: "second(" + t`column` + ")",
+ description: t`Takes a datetime and returns an integer (0-59) with the number of the seconds in the minute.`,
+ example: "second([" + t`Created At` + "])",
+ args: [
+ {
+ name: t`column`,
+ description: t`The datetime column.`,
+ },
+ ],
+ },
];
export const getHelpText = (name: string): HelpText | undefined => {
diff --git a/frontend/test/metabase/scenarios/custom-column/cc-data-type.cy.spec.js b/frontend/test/metabase/scenarios/custom-column/cc-data-type.cy.spec.js
index 0b5f94226a5..f3e70629074 100644
--- a/frontend/test/metabase/scenarios/custom-column/cc-data-type.cy.spec.js
+++ b/frontend/test/metabase/scenarios/custom-column/cc-data-type.cy.spec.js
@@ -4,6 +4,9 @@ import {
popover,
enterCustomColumnDetails,
filter,
+ openOrdersTable,
+ visualize,
+ getNotebookStep,
} from "__support__/e2e/helpers";
import { SAMPLE_DATABASE } from "__support__/e2e/cypress_sample_database";
@@ -34,6 +37,23 @@ describe("scenarios > question > custom column > data type", () => {
cy.findByPlaceholderText("Enter some text");
});
+ it("should understand date functions", () => {
+ openOrdersTable({ mode: "notebook" });
+
+ addCustomColumns([
+ { name: "Year", formula: "year([Created At])" },
+ { name: "Quarter", formula: "quarter([Created At])" },
+ { name: "Month", formula: "month([Created At])" },
+ { name: "Day", formula: "day([Created At])" },
+ { name: "Weekday", formula: "weekday([Created At])" },
+ { name: "Hour", formula: "hour([Created At])" },
+ { name: "Minute", formula: "minute([Created At])" },
+ { name: "Second", formula: "second([Created At])" },
+ ]);
+
+ visualize();
+ });
+
it("should relay the type of a date field", () => {
openCustomColumnInTable(PEOPLE_ID);
@@ -89,6 +109,19 @@ describe("scenarios > question > custom column > data type", () => {
});
});
+function addCustomColumns(columns) {
+ cy.wrap(columns).each((column, index) => {
+ if (index) {
+ getNotebookStep("expression").within(() => cy.icon("add").click());
+ } else {
+ cy.findByText("Custom column").click();
+ }
+
+ enterCustomColumnDetails(column);
+ cy.button("Done").click();
+ });
+}
+
function openCustomColumnInTable(table) {
openTable({ table, mode: "notebook" });
cy.findByText("Custom column").click();
diff --git a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj
index eff072fb28d..3461503c7a4 100644
--- a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj
+++ b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj
@@ -371,18 +371,20 @@
;; for datetimes or anything without a known temporal type, cast to timestamp and go from there
(recur unit (->temporal-type :timestamp expr))))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :minute] [_ _ expr] (trunc :minute expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :minute-of-hour] [_ _ expr] (extract :minute expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :hour] [_ _ expr] (trunc :hour expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :hour-of-day] [_ _ expr] (extract :hour expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :day] [_ _ expr] (trunc :day expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :day-of-month] [_ _ expr] (extract :day expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :day-of-year] [_ _ expr] (extract :dayofyear expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :month] [_ _ expr] (trunc :month expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :month-of-year] [_ _ expr] (extract :month expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :quarter] [_ _ expr] (trunc :quarter expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :quarter-of-year] [_ _ expr] (extract :quarter expr))
-(defmethod sql.qp/date [:bigquery-cloud-sdk :year] [_ _ expr] (trunc :year expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :second-of-minute] [_ _ expr] (extract :second expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :minute] [_ _ expr] (trunc :minute expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :minute-of-hour] [_ _ expr] (extract :minute expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :hour] [_ _ expr] (trunc :hour expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :hour-of-day] [_ _ expr] (extract :hour expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :day] [_ _ expr] (trunc :day expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :day-of-month] [_ _ expr] (extract :day expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :day-of-year] [_ _ expr] (extract :dayofyear expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :month] [_ _ expr] (trunc :month expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :month-of-year] [_ _ expr] (extract :month expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :quarter] [_ _ expr] (trunc :quarter expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :quarter-of-year] [_ _ expr] (extract :quarter expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :year] [_ _ expr] (trunc :year expr))
+(defmethod sql.qp/date [:bigquery-cloud-sdk :year-of-era] [_ _ expr] (extract :year expr))
;; BigQuery mod is a function like mod(x, y) rather than an operator like x mod y
(defmethod hformat/fn-handler (u/qualified-name ::mod)
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo.clj b/modules/drivers/mongo/src/metabase/driver/mongo.clj
index e4fd88c9730..84c511e066e 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo.clj
@@ -156,14 +156,14 @@
(defn- describe-table-field [field-kw field-info idx]
(let [most-common-object-type (most-common-object-type (vec (:types field-info)))
[nested-fields idx-next]
- (reduce
- (fn [[nested-fields idx] nested-field]
- (let [[nested-field idx-next] (describe-table-field nested-field
- (nested-field (:nested-fields field-info))
- idx)]
- [(conj nested-fields nested-field) idx-next]))
- [#{} (inc idx)]
- (keys (:nested-fields field-info)))]
+ (reduce
+ (fn [[nested-fields idx] nested-field]
+ (let [[nested-field idx-next] (describe-table-field nested-field
+ (nested-field (:nested-fields field-info))
+ idx)]
+ [(conj nested-fields nested-field) idx-next]))
+ [#{} (inc idx)]
+ (keys (:nested-fields field-info)))]
[(cond-> {:name (name field-kw)
:database-type (some-> most-common-object-type .getName)
:base-type (class->base-type most-common-object-type)
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj b/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
index f44c4f80bca..ef8a4aba035 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
@@ -21,8 +21,8 @@
[metabase.util.schema :as su]
[monger.operators :refer [$add $addToSet $and $avg $cond $dayOfMonth $dayOfWeek $dayOfYear $divide $eq
$group $gt $gte $hour $limit $lt $lte $match $max $min $minute $mod $month
- $multiply $ne $not $or $project $regex $size $skip $sort $strcasecmp $subtract
- $sum $toLower]]
+ $multiply $ne $not $or $project $regex $second $size $skip $sort $strcasecmp $subtract
+ $sum $toLower $year]]
[schema.core :as s])
(:import org.bson.types.ObjectId))
@@ -200,20 +200,21 @@
(letfn [(truncate [unit]
(truncate-to-resolution column unit))]
(case unit
- :default column
- :minute (truncate :minute)
- :minute-of-hour {$minute column}
- :hour (truncate :hour)
- :hour-of-day {$hour column}
- :day (truncate :day)
- :day-of-week (day-of-week column)
- :day-of-month {$dayOfMonth column}
- :day-of-year {$dayOfYear column}
- :week (truncate-to-resolution (week column) :day)
- :week-of-year {:$ceil {$divide [{$dayOfYear (week column)}
- 7.0]}}
- :month (truncate :month)
- :month-of-year {$month column}
+ :default column
+ :second-of-minute {$second column}
+ :minute (truncate :minute)
+ :minute-of-hour {$minute column}
+ :hour (truncate :hour)
+ :hour-of-day {$hour column}
+ :day (truncate :day)
+ :day-of-week (day-of-week column)
+ :day-of-month {$dayOfMonth column}
+ :day-of-year {$dayOfYear column}
+ :week (truncate-to-resolution (week column) :day)
+ :week-of-year {:$ceil {$divide [{$dayOfYear (week column)}
+ 7.0]}}
+ :month (truncate :month)
+ :month-of-year {$month column}
;; For quarter we'll just subtract enough days from the current date to put it in the correct month and
;; stringify it as yyyy-MM Subtracting (($dayOfYear(column) % 91) - 3) days will put you in correct month.
;; Trust me.
@@ -233,7 +234,10 @@
3]})
:year
- (truncate :year))))))
+ (truncate :year)
+
+ :year-of-era
+ {$year column})))))
(defmethod ->rvalue :field
[[_ id-or-name {:keys [temporal-unit]}]]
@@ -370,6 +374,20 @@
(defmethod ->rvalue :concat [[_ & args]] {"$concat" (mapv ->rvalue args)})
(defmethod ->rvalue :substring [[_ & args]] {"$substrCP" (mapv ->rvalue args)})
+(def ^:private temporal-extract-unit->date-unit
+ {:second :second-of-minute
+ :minute :minute-of-hour
+ :hour :hour-of-day
+ :day-of-week :day-of-week
+ :day :day-of-month
+ :week :week-of-year
+ :month :month-of-year
+ :quarter :quarter-of-year
+ :year :year-of-era})
+
+(defmethod ->rvalue :temporal-extract [[_ inp unit]]
+ (with-rvalue-temporal-bucketing (->rvalue inp) (temporal-extract-unit->date-unit unit)))
+
;;; Intervals are not first class Mongo citizens, so they cannot be translated on their own.
;;; The only thing we can do with them is adding to or subtracting from a date valued expression.
;;; Also, date arithmetic with intervals was first implemented in version 5. (Before that only
diff --git a/modules/drivers/oracle/src/metabase/driver/oracle.clj b/modules/drivers/oracle/src/metabase/driver/oracle.clj
index 0de45ed0b2a..dd92a6e76bd 100644
--- a/modules/drivers/oracle/src/metabase/driver/oracle.clj
+++ b/modules/drivers/oracle/src/metabase/driver/oracle.clj
@@ -157,18 +157,25 @@
;; trunc() returns a date -- see https://docs.oracle.com/cd/E11882_01/server.112/e10729/ch4datetime.htm#NLSPG253
(hx/with-database-type-info "date")))
-(defmethod sql.qp/date [:oracle :minute] [_ _ v] (trunc :mi v))
+(defmethod sql.qp/date [:oracle :second-of-minute] [_ _ v] (->> v
+ hx/->timestamp
+ (hsql/call :extract :second)
+ (hsql/call :floor)
+ hx/->integer))
+
+(defmethod sql.qp/date [:oracle :minute] [_ _ v] (trunc :mi v))
;; you can only extract minute + hour from TIMESTAMPs, even though DATEs still have them (WTF), so cast first
-(defmethod sql.qp/date [:oracle :minute-of-hour] [_ _ v] (hsql/call :extract :minute (hx/->timestamp v)))
-(defmethod sql.qp/date [:oracle :hour] [_ _ v] (trunc :hh v))
-(defmethod sql.qp/date [:oracle :hour-of-day] [_ _ v] (hsql/call :extract :hour (hx/->timestamp v)))
-(defmethod sql.qp/date [:oracle :day] [_ _ v] (trunc :dd v))
-(defmethod sql.qp/date [:oracle :day-of-month] [_ _ v] (hsql/call :extract :day v))
+(defmethod sql.qp/date [:oracle :minute-of-hour] [_ _ v] (hsql/call :extract :minute (hx/->timestamp v)))
+(defmethod sql.qp/date [:oracle :hour] [_ _ v] (trunc :hh v))
+(defmethod sql.qp/date [:oracle :hour-of-day] [_ _ v] (hsql/call :extract :hour (hx/->timestamp v)))
+(defmethod sql.qp/date [:oracle :day] [_ _ v] (trunc :dd v))
+(defmethod sql.qp/date [:oracle :day-of-month] [_ _ v] (hsql/call :extract :day v))
;; [SIC] The format template for truncating to start of week is 'day' in Oracle #WTF
-(defmethod sql.qp/date [:oracle :month] [_ _ v] (trunc :month v))
-(defmethod sql.qp/date [:oracle :month-of-year] [_ _ v] (hsql/call :extract :month v))
-(defmethod sql.qp/date [:oracle :quarter] [_ _ v] (trunc :q v))
-(defmethod sql.qp/date [:oracle :year] [_ _ v] (trunc :year v))
+(defmethod sql.qp/date [:oracle :month] [_ _ v] (trunc :month v))
+(defmethod sql.qp/date [:oracle :month-of-year] [_ _ v] (hsql/call :extract :month v))
+(defmethod sql.qp/date [:oracle :quarter] [_ _ v] (trunc :q v))
+(defmethod sql.qp/date [:oracle :year] [_ _ v] (trunc :year v))
+(defmethod sql.qp/date [:oracle :year-of-era] [_ _ v] (hsql/call :extract :year v))
(defmethod sql.qp/date [:oracle :week]
[driver _ v]
diff --git a/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj b/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj
index ebd1baaacb0..61317822ec1 100644
--- a/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj
+++ b/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj
@@ -26,20 +26,20 @@
(deftest describe-database-test
(mt/test-driver :presto-jdbc
- (is (= {:tables #{{:name "categories" :schema "default"}
- {:name "venues" :schema "default"}
- {:name "checkins" :schema "default"}
- {:name "users" :schema "default"}}}
+ (is (= {:tables #{{:name "test_data_categories" :schema "default"}
+ {:name "test_data_venues" :schema "default"}
+ {:name "test_data_checkins" :schema "default"}
+ {:name "test_data_users" :schema "default"}}}
(-> (driver/describe-database :presto-jdbc (mt/db))
- (update :tables (comp set (partial filter (comp #{"categories"
- "venues"
- "checkins"
- "users"}
+ (update :tables (comp set (partial filter (comp #{"test_data_categories"
+ "test_data_venues"
+ "test_data_checkins"
+ "test_data_users"}
:name)))))))))
(deftest describe-table-test
(mt/test-driver :presto-jdbc
- (is (= {:name "venues"
+ (is (= {:name "test_data_venues"
:schema "default"
:fields #{{:name "name",
;; for HTTP based Presto driver, this is coming back as varchar(255)
@@ -135,8 +135,8 @@
:filter [:= $name "wow"]})]
(testing "The native query returned in query results should use user-friendly splicing"
(is (= (str "SELECT count(*) AS \"count\" "
- "FROM \"default\".\"venues\" "
- "WHERE \"default\".\"venues\".\"name\" = 'wow'")
+ "FROM \"default\".\"test_data_venues\" "
+ "WHERE \"default\".\"test_data_venues\".\"name\" = 'wow'")
(:query (qp/compile-and-splice-parameters query))
(-> (qp/process-query query) :data :native_form :query)))))))
diff --git a/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj b/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
index 84fe3abce21..ab99a02c6dd 100644
--- a/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
+++ b/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
@@ -24,6 +24,12 @@
;; during unit tests don't treat presto as having FK support
(defmethod driver/supports? [:presto-jdbc :foreign-keys] [_ _] (not config/is-test?))
+;; in the past, we had to manually update our Docker image and add a new catalog for every new dataset definition we
+;; added. That's insane. Just use the `test-data` catalog and put everything in that, and use
+;; `db-qualified-table-name` like everyone else.
+(def ^:private test-catalog-name "test_data")
+
+
(doseq [[base-type db-type] {:type/BigInteger "BIGINT"
:type/Boolean "BOOLEAN"
:type/Date "DATE"
@@ -39,7 +45,7 @@
:type/TimeWithTZ "TIME WITH TIME ZONE"}]
(defmethod sql.tx/field-base-type->sql-type [:presto-jdbc base-type] [_ _] db-type))
-(defn dbdef->connection-details [database-name]
+(defn dbdef->connection-details [_database-name]
(let [base-details
{:host (tx/db-test-env-var-or-throw :presto-jdbc :host "localhost")
:port (tx/db-test-env-var :presto-jdbc :port "8080")
@@ -58,7 +64,7 @@
:kerberos-keytab-path (tx/db-test-env-var :presto-jdbc :kerberos-keytab-path nil)
:kerberos-config-path (tx/db-test-env-var :presto-jdbc :kerberos-config-path nil)
:kerberos-service-principal-pattern (tx/db-test-env-var :presto-jdbc :kerberos-service-principal-pattern nil)
- :catalog (u/snake-key database-name)
+ :catalog test-catalog-name
:schema (tx/db-test-env-var :presto-jdbc :schema nil)}]
(assoc base-details
:ssl-use-keystore (every? some? (map base-details [:ssl-keystore-path :ssl-keystore-password-value]))
@@ -124,9 +130,9 @@
(defmethod sql.tx/qualified-name-components :presto-jdbc
;; use the default schema from the in-memory connector
- ([_ db-name] [(u/snake-key db-name) "default"])
- ([_ db-name table-name] [(u/snake-key db-name) "default" (u/snake-key table-name)])
- ([_ db-name table-name field-name] [(u/snake-key db-name) "default" (u/snake-key table-name) field-name]))
+ ([_ _db-name] [test-catalog-name "default"])
+ ([_ db-name table-name] [test-catalog-name "default" (tx/db-qualified-table-name db-name table-name)])
+ ([_ db-name table-name field-name] [test-catalog-name "default" (tx/db-qualified-table-name db-name table-name) field-name]))
(defmethod sql.tx/pk-sql-type :presto-jdbc
[_]
diff --git a/modules/drivers/presto/src/metabase/driver/presto.clj b/modules/drivers/presto/src/metabase/driver/presto.clj
index 8d9ff111618..7d462473bdb 100644
--- a/modules/drivers/presto/src/metabase/driver/presto.clj
+++ b/modules/drivers/presto/src/metabase/driver/presto.clj
@@ -9,6 +9,7 @@
[metabase.driver :as driver]
[metabase.driver.presto-common :as presto-common]
[metabase.driver.sql-jdbc.sync.describe-database :as sql-jdbc.describe-database]
+ [metabase.driver.sql.query-processor :as sql.qp]
[metabase.driver.sql.util :as sql.u]
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.query-processor.context :as qp.context]
@@ -17,6 +18,7 @@
[metabase.query-processor.util :as qp.util]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
+ [metabase.util.honeysql-extensions :as hx]
[metabase.util.i18n :refer [trs tru]]
[metabase.util.schema :as su]
[metabase.util.ssh :as ssh]
@@ -24,6 +26,14 @@
(driver/register! :presto, :parent :presto-common)
+(defmethod sql.qp/cast-temporal-string [:presto :Coercion/ISO8601->DateTime]
+ [_ _coercion-strategy expr]
+ (hx/->timestamp expr))
+
+(defmethod sql.qp/cast-temporal-string [:presto :Coercion/ISO8601->Date]
+ [_ _coercion-strategy expr]
+ (hx/->date expr))
+
;;; Presto API helpers
(def ^:private PrestoConnectionDetails
diff --git a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj
index 97eda18cb28..1695b1fd422 100644
--- a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj
+++ b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj
@@ -128,6 +128,10 @@
[driver _ expr]
(->datetime (strftime "%Y-%m-%d %H:%M:%S" (sql.qp/->honeysql driver expr))))
+(defmethod sql.qp/date [:sqlite :second-of-minute]
+ [driver _ expr]
+ (hx/->integer (strftime "%S" (sql.qp/->honeysql driver expr))))
+
(defmethod sql.qp/date [:sqlite :minute]
[driver _ expr]
(->datetime (strftime "%Y-%m-%d %H:%M" (sql.qp/->honeysql driver expr))))
@@ -206,6 +210,10 @@
[driver _ expr]
(->date (sql.qp/->honeysql driver expr) (hx/literal "start of year")))
+(defmethod sql.qp/date [:sqlite :year-of-era]
+ [driver _ expr]
+ (hx/->integer (strftime "%Y" (sql.qp/->honeysql driver expr))))
+
(defmethod sql.qp/add-interval-honeysql-form :sqlite
[_driver hsql-form amount unit]
(let [[multiplier sqlite-unit] (case unit
diff --git a/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj b/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj
index a0cde770165..2d521f830d2 100644
--- a/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj
+++ b/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj
@@ -128,6 +128,10 @@
[_ _ expr]
expr)
+(defmethod sql.qp/date [:sqlserver :second-of-minute]
+ [_ _ expr]
+ (date-part :second expr))
+
(defmethod sql.qp/date [:sqlserver :minute]
[_ _ expr]
(hx/maybe-cast :smalldatetime expr))
@@ -203,6 +207,10 @@
(hx/year expr)
(hsql/call :DateFromParts (hx/year expr) 1 1)))
+(defmethod sql.qp/date [:sqlserver :year-of-era]
+ [_ _ expr]
+ (date-part :year expr))
+
(defmethod sql.qp/add-interval-honeysql-form :sqlserver
[_ hsql-form amount unit]
(date-add unit amount hsql-form))
diff --git a/shared/src/metabase/mbql/normalize.cljc b/shared/src/metabase/mbql/normalize.cljc
index b89ff8d52df..ebb400e2c24 100644
--- a/shared/src/metabase/mbql/normalize.cljc
+++ b/shared/src/metabase/mbql/normalize.cljc
@@ -149,6 +149,10 @@
[[_ amount unit]]
[:interval amount (maybe-normalize-token unit)])
+(defmethod normalize-mbql-clause-tokens :temporal-extract
+ [[_ field unit]]
+ [:temporal-extract (normalize-tokens field :ignore-path) (maybe-normalize-token unit)])
+
(defmethod normalize-mbql-clause-tokens :value
;; The args of a `value` clause shouldn't be normalized.
;; See https://github.com/metabase/metabase/issues/23354 for details
diff --git a/shared/src/metabase/mbql/predicates.cljc b/shared/src/metabase/mbql/predicates.cljc
index 019982be640..1c6788bb161 100644
--- a/shared/src/metabase/mbql/predicates.cljc
+++ b/shared/src/metabase/mbql/predicates.cljc
@@ -29,3 +29,7 @@
(def ^{:arglists '([filter-clause])} Filter?
"Is this a valid `:filter` clause?"
(complement (s/checker mbql.s/Filter)))
+
+(def ^{:arglists '([filter-clause])} DatetimeExpression?
+ "Is this a valid DatetimeExpression clause?"
+ (complement (s/checker mbql.s/DatetimeExpression)))
diff --git a/shared/src/metabase/mbql/schema.cljc b/shared/src/metabase/mbql/schema.cljc
index dfc6ced8279..2ab06654bfc 100644
--- a/shared/src/metabase/mbql/schema.cljc
+++ b/shared/src/metabase/mbql/schema.cljc
@@ -78,6 +78,12 @@
(apply s/enum datetime-bucketing-units)
"datetime-bucketing-unit"))
+(def TemporalExtractUnits
+ "Valid units to extract from a temporal."
+ (s/named
+ (apply s/enum #{:second :minute :hour :day :day-of-week :week :month :quarter :year})
+ "temporal-extract-units"))
+
(def ^:private RelativeDatetimeUnit
(s/named
(apply s/enum #{:default :minute :hour :day :week :month :quarter :year})
@@ -462,8 +468,20 @@
(def ^:private aggregations #{:sum :avg :stddev :var :median :percentile :min :max :cum-count :cum-sum :count-where :sum-where :share :distinct :metric :aggregation-options :count})
+(def temporal-extract-functions
+ "Functions to extract components of a date, datetime."
+ #{;; extraction functions (get some component of a given temporal value/column)
+ :temporal-extract
+ ;; SUGAR drivers do not need to implement
+ :get-year :get-quarter :get-month :get-day :get-day-of-week :get-hour :get-minute :get-second})
+
+(def date+time+timezone-functions
+ "Date, time, and timezone related functions."
+ (set/union temporal-extract-functions))
+
(declare ArithmeticExpression)
(declare BooleanExpression)
+(declare DatetimeExpression)
(declare Aggregation)
(def ^:private NumericExpressionArg
@@ -474,6 +492,9 @@
(partial is-clause? arithmetic-expressions)
(s/recursive #'ArithmeticExpression)
+ (partial is-clause? temporal-extract-functions)
+ (s/recursive #'DatetimeExpression)
+
(partial is-clause? aggregations)
(s/recursive #'Aggregation)
@@ -483,6 +504,17 @@
:else
Field))
+(def ^:private DateTimeExpressionArg
+ (s/conditional
+ (partial is-clause? aggregations)
+ (s/recursive #'Aggregation)
+
+ (partial is-clause? :value)
+ value
+
+ :else
+ Field))
+
(def ^:private ExpressionArg
(s/conditional
number?
@@ -503,6 +535,9 @@
(partial is-clause? string-expressions)
(s/recursive #'StringExpression)
+ (partial is-clause? temporal-extract-functions)
+ (s/recursive #'DatetimeExpression)
+
(partial is-clause? :value)
value
@@ -587,13 +622,51 @@
"Schema for the definition of an arithmetic expression."
(s/recursive #'ArithmeticExpression*))
+(defclause ^{:requires-features #{:temporal-extract}} temporal-extract
+ datetime DateTimeExpressionArg
+ unit TemporalExtractUnits)
+
+;; SUGAR CLAUSE: get-year, get-month... clauses are all sugars clause that will be rewritten as [:temporal-extract column :year]
+(defclause ^{:requires-features #{:temporal-extract}} ^:sugar get-year
+ date DateTimeExpressionArg)
+
+(defclause ^{:requires-features #{:temporal-extract}} ^:sugar get-quarter
+ date DateTimeExpressionArg)
+
+(defclause ^{:requires-features #{:temporal-extract}} ^:sugar get-month
+ date DateTimeExpressionArg)
+
+(defclause ^{:requires-features #{:temporal-extract}} ^:sugar get-day
+ date DateTimeExpressionArg)
+
+(defclause ^{:requires-features #{:temporal-extract}} ^:sugar get-day-of-week
+ date DateTimeExpressionArg)
+
+(defclause ^{:requires-features #{:temporal-extract}} ^:sugar get-hour
+ datetime DateTimeExpressionArg)
+
+(defclause ^{:requires-features #{:temporal-extract}} ^:sugar get-minute
+ datetime DateTimeExpressionArg)
+
+(defclause ^{:requires-features #{:temporal-extract}} ^:sugar get-second
+ datetime DateTimeExpressionArg)
+
+(def ^:private DatetimeExpression*
+ (one-of temporal-extract
+ ;; SUGAR drivers do not need to implement
+ get-year get-quarter get-month get-day get-day-of-week get-hour
+ get-minute get-second))
+
+(def DatetimeExpression
+ "Schema for the definition of a date function expression."
+ (s/recursive #'DatetimeExpression*))
+
(declare StringExpression*)
(def ^:private StringExpression
"Schema for the definition of an string expression."
(s/recursive #'StringExpression*))
-
;;; ----------------------------------------------------- Filter -----------------------------------------------------
(declare Filter)
@@ -769,18 +842,18 @@
"Schema for anything that is accepted as a top-level expression definition, either an arithmetic expression such as a
`:+` clause or a `:field` clause."
(s/conditional
- (partial is-clause? arithmetic-expressions) ArithmeticExpression
- (partial is-clause? string-expressions) StringExpression
- (partial is-clause? boolean-expressions) BooleanExpression
- (partial is-clause? :case) case
- :else Field))
-
+ (partial is-clause? arithmetic-expressions) ArithmeticExpression
+ (partial is-clause? string-expressions) StringExpression
+ (partial is-clause? boolean-expressions) BooleanExpression
+ (partial is-clause? date+time+timezone-functions) DatetimeExpression
+ (partial is-clause? :case) case
+ :else Field))
;;; -------------------------------------------------- Aggregations --------------------------------------------------
;; For all of the 'normal' Aggregations below (excluding Metrics) fields are implicit Field IDs
-;; cum-sum and cum-count are SUGAR because they're implemented in middleware. They clauses are swapped out with
+;; cum-sum and cum-count are SUGAR because they're implemented in middleware. The clauses are swapped out with
;; `count` and `sum` aggregations respectively and summation is done in Clojure-land
(defclause ^{:requires-features #{:basic-aggregations}} ^:sugar count, field (optional Field))
(defclause ^{:requires-features #{:basic-aggregations}} ^:sugar cum-count, field (optional Field))
@@ -837,8 +910,11 @@
(def ^:private UnnamedAggregation*
(s/if (partial is-clause? arithmetic-expressions)
ArithmeticExpression
- (one-of count avg cum-count cum-sum distinct stddev sum min max metric share count-where
- sum-where case median percentile ag:var)))
+ (one-of avg cum-sum distinct stddev sum min max metric share count-where
+ sum-where case median percentile ag:var
+ ;; SUGAR clauses
+ cum-count count)))
+
(def ^:private UnnamedAggregation
(s/recursive #'UnnamedAggregation*))
@@ -1348,8 +1424,8 @@
:number/between {:type :numeric, :operator :binary, :allowed-for #{:number/between}}
:string/!= {:type :string, :operator :variadic, :allowed-for #{:string/!=}}
:string/= {:type :string, :operator :variadic, :allowed-for #{:string/= :text :id :category
- :location/city :location/state
- :location/zip_code :location/country}}
+ :location/city :location/state
+ :location/zip_code :location/country}}
:string/contains {:type :string, :operator :unary, :allowed-for #{:string/contains}}
:string/does-not-contain {:type :string, :operator :unary, :allowed-for #{:string/does-not-contain}}
:string/ends-with {:type :string, :operator :unary, :allowed-for #{:string/ends-with}}
diff --git a/shared/src/metabase/mbql/util.cljc b/shared/src/metabase/mbql/util.cljc
index 8ea82b2062a..e4b3a9901ba 100644
--- a/shared/src/metabase/mbql/util.cljc
+++ b/shared/src/metabase/mbql/util.cljc
@@ -250,6 +250,27 @@
[:relative-datetime :current]
[:relative-datetime 0 temporal-unit])))))
+(def temporal-extract-ops->unit
+ "Mapping from the sugar syntax to extract datetime to the unit."
+ {:get-year :year
+ :get-quarter :quarter
+ :get-month :month
+ :get-day :day
+ :get-day-of-week :day-of-week
+ :get-hour :hour
+ :get-minute :minute
+ :get-second :second})
+
+(def ^:private temporal-extract-ops
+ (set (keys temporal-extract-ops->unit)))
+
+(defn desugar-temporal-extract
+ "Replace datetime extractions clauses like `[:get-year field]` with `[:temporal-extract field :year]`."
+ [m]
+ (mbql.match/replace m
+ [(op :guard temporal-extract-ops) field]
+ [:temporal-extract field (temporal-extract-ops->unit op)]))
+
(s/defn desugar-filter-clause :- mbql.s/Filter
"Rewrite various 'syntatic sugar' filter clauses like `:time-interval` and `:inside` as simpler, logically
equivalent clauses. This can be used to simplify the number of filter clauses that need to be supported by anything
@@ -264,7 +285,8 @@
desugar-is-null-and-not-null
desugar-is-empty-and-not-empty
desugar-inside
- simplify-compound-filter))
+ simplify-compound-filter
+ desugar-temporal-extract))
(defmulti ^:private negate* first)
diff --git a/shared/test/metabase/mbql/util_test.cljc b/shared/test/metabase/mbql/util_test.cljc
index 255c3ce7028..6ef0fc41b14 100644
--- a/shared/test/metabase/mbql/util_test.cljc
+++ b/shared/test/metabase/mbql/util_test.cljc
@@ -362,6 +362,15 @@
(t/is (= [:not [:contains [:field 1 nil] "ABC" {:case-sensitive false}]]
(mbql.u/desugar-filter-clause [:does-not-contain [:field 1 nil] "ABC" {:case-sensitive false}])))))
+(t/deftest ^:parallel desugar-temporal-extract-test
+ (t/testing "desugaring :get-year, :get-month, etc"
+ (doseq [[op unit] mbql.u/temporal-extract-ops->unit]
+ (t/is (= [:temporal-extract [:field 1 nil] unit]
+ (mbql.u/desugar-temporal-extract [op [:field 1 nil]])))
+
+ (t/is (= [:+ [:temporal-extract [:field 1 nil] unit] 1]
+ (mbql.u/desugar-temporal-extract [:+ [op [:field 1 nil]] 1]))))))
+
(t/deftest ^:parallel negate-simple-filter-clause-test
(t/testing :=
(t/is (= [:!= [:field 1 nil] 10]
@@ -564,12 +573,12 @@
(t/testing "ok, can we do the same thing as the tests above but make those names *unique* at the same time?"
- (t/is (= [[:aggregation-options [:sum [:field 1 nil]] {:name "sum"} ]
+ (t/is (= [[:aggregation-options [:sum [:field 1 nil]] {:name "sum"}]
[:aggregation-options [:count [:field 1 nil]] {:name "count"}]
[:aggregation-options [:sum [:field 1 nil]] {:name "sum_2"}]
- [:aggregation-options [:avg [:field 1 nil]] {:name "avg"} ]
+ [:aggregation-options [:avg [:field 1 nil]] {:name "avg"}]
[:aggregation-options [:sum [:field 1 nil]] {:name "sum_3"}]
- [:aggregation-options [:min [:field 1 nil]] {:name "min"} ]]
+ [:aggregation-options [:min [:field 1 nil]] {:name "min"}]]
(mbql.u/pre-alias-and-uniquify-aggregations simple-ag->name
[[:sum [:field 1 nil]]
[:count [:field 1 nil]]
@@ -701,7 +710,7 @@
"if nothing is set return `nil`"
{{:database 1
:type :query
- :query {:source-table 1}} nil}} ]
+ :query {:source-table 1}} nil}}]
(t/testing group
(doseq [[query expected] query->expected]
(t/testing (pr-str (list 'query->max-rows-limit query))
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 6b53e1342ba..5f37b241735 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -436,6 +436,10 @@
;; Does the driver support percentile calculations (including median)
:percentile-aggregations
+ ;; Does the driver support date, time, and timezone manipulation functions?
+ ;; DEFAULTS TO TRUE
+ :temporal-extract
+
;; Does the driver support experimental "writeback" actions like "delete this row" or "insert a new row" from 44+?
:actions
@@ -462,6 +466,7 @@
(defmethod supports? [::driver :basic-aggregations] [_ _] true)
(defmethod supports? [::driver :case-sensitivity-string-filter-options] [_ _] true)
+(defmethod supports? [::driver :temporal-extract] [_ _] true)
(defmulti database-supports?
"Does this driver and specific instance of a database support a certain `feature`?
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index e730295e514..cf7bf21f281 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -245,19 +245,21 @@
(def ^:private extract-integer (comp hx/->integer extract))
-(defmethod sql.qp/date [:postgres :default] [_ _ expr] expr)
-(defmethod sql.qp/date [:postgres :minute] [_ _ expr] (date-trunc :minute expr))
-(defmethod sql.qp/date [:postgres :minute-of-hour] [_ _ expr] (extract-integer :minute expr))
-(defmethod sql.qp/date [:postgres :hour] [_ _ expr] (date-trunc :hour expr))
-(defmethod sql.qp/date [:postgres :hour-of-day] [_ _ expr] (extract-integer :hour expr))
-(defmethod sql.qp/date [:postgres :day] [_ _ expr] (hx/->date expr))
-(defmethod sql.qp/date [:postgres :day-of-month] [_ _ expr] (extract-integer :day expr))
-(defmethod sql.qp/date [:postgres :day-of-year] [_ _ expr] (extract-integer :doy expr))
-(defmethod sql.qp/date [:postgres :month] [_ _ expr] (date-trunc :month expr))
-(defmethod sql.qp/date [:postgres :month-of-year] [_ _ expr] (extract-integer :month expr))
-(defmethod sql.qp/date [:postgres :quarter] [_ _ expr] (date-trunc :quarter expr))
-(defmethod sql.qp/date [:postgres :quarter-of-year] [_ _ expr] (extract-integer :quarter expr))
-(defmethod sql.qp/date [:postgres :year] [_ _ expr] (date-trunc :year expr))
+(defmethod sql.qp/date [:postgres :default] [_ _ expr] expr)
+(defmethod sql.qp/date [:postgres :second-of-minute] [_ _ expr] (extract-integer :second expr))
+(defmethod sql.qp/date [:postgres :minute] [_ _ expr] (date-trunc :minute expr))
+(defmethod sql.qp/date [:postgres :minute-of-hour] [_ _ expr] (extract-integer :minute expr))
+(defmethod sql.qp/date [:postgres :hour] [_ _ expr] (date-trunc :hour expr))
+(defmethod sql.qp/date [:postgres :hour-of-day] [_ _ expr] (extract-integer :hour expr))
+(defmethod sql.qp/date [:postgres :day] [_ _ expr] (hx/->date expr))
+(defmethod sql.qp/date [:postgres :day-of-month] [_ _ expr] (extract-integer :day expr))
+(defmethod sql.qp/date [:postgres :day-of-year] [_ _ expr] (extract-integer :doy expr))
+(defmethod sql.qp/date [:postgres :month] [_ _ expr] (date-trunc :month expr))
+(defmethod sql.qp/date [:postgres :month-of-year] [_ _ expr] (extract-integer :month expr))
+(defmethod sql.qp/date [:postgres :quarter] [_ _ expr] (date-trunc :quarter expr))
+(defmethod sql.qp/date [:postgres :quarter-of-year] [_ _ expr] (extract-integer :quarter expr))
+(defmethod sql.qp/date [:postgres :year] [_ _ expr] (date-trunc :year expr))
+(defmethod sql.qp/date [:postgres :year-of-era] [_ _ expr] (extract-integer :year expr))
(defmethod sql.qp/date [:postgres :day-of-week]
[_ driver expr]
diff --git a/src/metabase/driver/sql/query_processor.clj b/src/metabase/driver/sql/query_processor.clj
index f740fa51b38..de69ec71f9c 100644
--- a/src/metabase/driver/sql/query_processor.clj
+++ b/src/metabase/driver/sql/query_processor.clj
@@ -99,6 +99,18 @@
[_driver]
:%now)
+(def temporal-extract-unit->date-unit
+ "Mapping from the unit we used in `extract` function to the unit we used for `date` function."
+ {:second :second-of-minute
+ :minute :minute-of-hour
+ :hour :hour-of-day
+ :day-of-week :day-of-week
+ :day :day-of-month
+ :week :week-of-year
+ :month :month-of-year
+ :quarter :quarter-of-year
+ :year :year-of-era})
+
;; TODO - rename this to `temporal-bucket` or something that better describes what it actually does
(defmulti date
"Return a HoneySQL form for truncating a date or timestamp field or value to a given resolution, or extracting a date
@@ -109,12 +121,17 @@
;; default implementation for `:default` bucketing returns expression as-is
(defmethod date [:sql :default] [_ _ expr] expr)
-
;; We have to roll our own to account for arbitrary start of week
-(defmethod date [:sql :week-of-year]
- [driver _ expr]
+
+(defmethod date [:sql :second-of-minute] [_driver _ expr] (hx/second expr))
+(defmethod date [:sql :minute-of-hour] [_driver _ expr] (hx/minute expr))
+(defmethod date [:sql :hour-of-day] [_driver _ expr] (hx/hour expr))
+(defmethod date [:sql :week-of-year] [driver _ expr]
;; Some DBs truncate when doing integer division, therefore force float arithmetics
(->honeysql driver [:ceil (hx// (date driver :day-of-year (date driver :week expr)) 7.0)]))
+(defmethod date [:sql :month-of-year] [_driver _ expr] (hx/month expr))
+(defmethod date [:sql :quarter-of-year] [_driver _ expr] (hx/quarter expr))
+(defmethod date [:sql :year-of-era] [_driver _ expr] (hx/year expr))
(defmulti add-interval-honeysql-form
"Return a HoneySQL form that performs represents addition of some temporal interval to the original `hsql-form`.
@@ -569,6 +586,9 @@
(current-datetime-honeysql-form driver)
(add-interval-honeysql-form driver (current-datetime-honeysql-form driver) amount unit))))
+(defmethod ->honeysql [:sql :temporal-extract]
+ [driver [_ arg unit]]
+ (date driver (temporal-extract-unit->date-unit unit) (->honeysql driver arg)))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Field Aliases (AS Forms) |
diff --git a/src/metabase/query_processor/middleware/desugar.clj b/src/metabase/query_processor/middleware/desugar.clj
index 34c3765c033..5c065dfb161 100644
--- a/src/metabase/query_processor/middleware/desugar.clj
+++ b/src/metabase/query_processor/middleware/desugar.clj
@@ -11,6 +11,9 @@
drivers need to support. Clauses replaced by this middleware are marked `^:sugar` in the MBQL schema."
[query]
(m/update-existing query :query (fn [query]
- (mbql.u/replace query
- (filter-clause :guard mbql.preds/Filter?)
- (mbql.u/desugar-filter-clause filter-clause)))))
+ (mbql.u/replace query
+ (filter-clause :guard mbql.preds/Filter?)
+ (mbql.u/desugar-filter-clause filter-clause)
+
+ (temporal-extract-clause :guard mbql.preds/DatetimeExpression?)
+ (mbql.u/desugar-temporal-extract temporal-extract-clause)))))
diff --git a/src/metabase/util/date_2.clj b/src/metabase/util/date_2.clj
index 3e27d2bc523..98a4c396122 100644
--- a/src/metabase/util/date_2.clj
+++ b/src/metabase/util/date_2.clj
@@ -190,7 +190,8 @@
;; well. Not sure where we'd use these, but we should have them for consistency
(def extract-units
"Units which return a (numerical, periodic) component of a date"
- #{:minute-of-hour
+ #{:second-of-minute
+ :minute-of-hour
:hour-of-day
:day-of-week
:day-of-month
@@ -234,6 +235,7 @@
([t :- Temporal, unit :- (apply s/enum extract-units)]
(t/as t (case unit
+ :second-of-minute :second-of-minute
:minute-of-hour :minute-of-hour
:hour-of-day :hour-of-day
:day-of-week (.dayOfWeek (week-fields (start-of-week)))
diff --git a/src/metabase/util/honeysql_extensions.clj b/src/metabase/util/honeysql_extensions.clj
index a37129f7646..8bf0ba677bc 100644
--- a/src/metabase/util/honeysql_extensions.clj
+++ b/src/metabase/util/honeysql_extensions.clj
@@ -1,5 +1,5 @@
(ns metabase.util.honeysql-extensions
- (:refer-clojure :exclude [+ - / * mod inc dec cast concat format])
+ (:refer-clojure :exclude [+ - / * mod inc dec cast concat format second])
(:require [clojure.pprint :as pprint]
[clojure.string :as str]
[honeysql.core :as hsql]
@@ -300,8 +300,9 @@
;;; Random SQL fns. Not all DBs support all these!
(def ^{:arglists '([& exprs])} floor "SQL `floor` function." (partial hsql/call :floor))
-(def ^{:arglists '([& exprs])} hour "SQL `hour` function." (partial hsql/call :hour))
+(def ^{:arglists '([& exprs])} second "SQL `second` function." (partial hsql/call :second))
(def ^{:arglists '([& exprs])} minute "SQL `minute` function." (partial hsql/call :minute))
+(def ^{:arglists '([& exprs])} hour "SQL `hour` function." (partial hsql/call :hour))
(def ^{:arglists '([& exprs])} day "SQL `day` function." (partial hsql/call :day))
(def ^{:arglists '([& exprs])} week "SQL `week` function." (partial hsql/call :week))
(def ^{:arglists '([& exprs])} month "SQL `month` function." (partial hsql/call :month))
diff --git a/test/metabase/automagic_dashboards/core_test.clj b/test/metabase/automagic_dashboards/core_test.clj
index 8b21ff15cfb..ff2a513e521 100644
--- a/test/metabase/automagic_dashboards/core_test.clj
+++ b/test/metabase/automagic_dashboards/core_test.clj
@@ -514,7 +514,7 @@
(deftest handlers-test
(testing "Make sure we have handlers for all the units available"
(doseq [unit (disj (set (concat u.date/extract-units u.date/truncate-units))
- :iso-day-of-year :millisecond)]
+ :iso-day-of-year :second-of-minute :millisecond)]
(testing unit
(is (some? (#'magic/humanize-datetime "1990-09-09T12:30:00" unit)))))))
diff --git a/test/metabase/query_processor/middleware/desugar_test.clj b/test/metabase/query_processor/middleware/desugar_test.clj
index 4fc00f14ed4..658f84a2944 100644
--- a/test/metabase/query_processor/middleware/desugar_test.clj
+++ b/test/metabase/query_processor/middleware/desugar_test.clj
@@ -14,7 +14,9 @@
[:relative-datetime -30 :day]
[:relative-datetime -1 :day]]
[:!= [:field 3 nil] "(not set)"]
- [:!= [:field 3 nil] "url"]]
+ [:!= [:field 3 nil] "url"]
+ [:> [:temporal-extract [:field 4 nil] :year] 2004]]
+ :expressions {"year" [:temporal-extract [:field 4 nil] :year]}
:aggregation [[:share [:and
[:= [:field 1 nil] "Run Query"]
[:between
@@ -30,7 +32,9 @@
:filter [:and
[:= [:field 1 nil] "Run Query"]
[:time-interval [:field 2 nil] -30 :day]
- [:!= [:field 3 nil] "(not set)" "url"]]
+ [:!= [:field 3 nil] "(not set)" "url"]
+ [:> [:get-year [:field 4 nil]] 2004]]
+ :expressions {"year" [:get-year [:field 4 nil]]}
:aggregation [[:share [:and
[:= [:field 1 nil] "Run Query"]
[:time-interval [:field 2 nil] -30 :day]
diff --git a/test/metabase/query_processor_test/date_time_zone_functions_test.clj b/test/metabase/query_processor_test/date_time_zone_functions_test.clj
new file mode 100644
index 00000000000..943425e0727
--- /dev/null
+++ b/test/metabase/query_processor_test/date_time_zone_functions_test.clj
@@ -0,0 +1,144 @@
+(ns metabase.query-processor-test.date-time-zone-functions-test
+ (:require [clojure.test :refer :all]
+ [metabase.driver :as driver]
+ [metabase.test :as mt]
+ [metabase.util.date-2 :as u.date]))
+
+(defn test-temporal-extract
+ [{:keys [aggregation breakout expressions fields filter limit]}]
+ (if breakout
+ (->> (mt/run-mbql-query times {:expressions expressions
+ :aggregation aggregation
+ :limit limit
+ :filter filter
+ :breakout breakout})
+ (mt/formatted-rows [int int]))
+ (->> (mt/run-mbql-query times {:expressions expressions
+ :aggregation aggregation
+ :limit limit
+ :filter filter
+ :fields fields})
+ (mt/formatted-rows [int])
+ (map first))))
+
+(mt/defdataset times-mixed
+ [["times" [{:field-name "index"
+ :base-type :type/Integer}
+ {:field-name "dt"
+ :base-type :type/DateTime}
+ {:field-name "d"
+ :base-type :type/Date}
+ {:field-name "as_dt"
+ :base-type :type/Text
+ :effective-type :type/DateTime
+ :coercion-strategy :Coercion/ISO8601->DateTime}
+ {:field-name "as_d"
+ :base-type :type/Text
+ :effective-type :type/Date
+ :coercion-strategy :Coercion/ISO8601->Date}]
+ [[1 #t "2004-03-19 09:19:09" #t "2004-03-19" "2004-03-19 09:19:09" "2004-03-19"]
+ [2 #t "2008-06-20 10:20:10" #t "2008-06-20" "2008-06-20 10:20:10" "2008-06-20"]
+ [3 #t "2012-11-21 11:21:11" #t "2012-11-21" "2012-11-21 11:21:11" "2012-11-21"]
+ [4 #t "2012-11-21 11:21:11" #t "2012-11-21" "2012-11-21 11:21:11" "2012-11-21"]]]])
+
+(def ^:private temporal-extraction-op->unit
+ {:get-second :second-of-minute
+ :get-minute :minute-of-hour
+ :get-hour :hour-of-day
+ :get-day-of-week :day-of-week
+ :get-day :day-of-month
+ :get-week :week-of-year
+ :get-month :month-of-year
+ :get-quarter :quarter-of-year
+ :get-year :year})
+
+(defn- extract
+ [x op]
+ (u.date/extract x (temporal-extraction-op->unit op)))
+
+(def ^:private extraction-test-cases
+ [{:expected-fn (fn [op] [(extract #t "2004-03-19 09:19:09" op) (extract #t "2008-06-20 10:20:10" op)
+ (extract #t "2012-11-21 11:21:11" op) (extract #t "2012-11-21 11:21:11" op)])
+ :query-fn (fn [op field-id] {:expressions {"expr" [op [:field field-id nil]]}
+ :fields [[:expression "expr"]]})}
+ {:expected-fn (fn [op] (into [] (frequencies [(extract #t "2004-03-19 09:19:09" op)
+ (extract #t "2008-06-20 10:20:10" op)
+ (extract #t "2012-11-21 11:21:11" op)
+ (extract #t "2012-11-21 11:21:11" op)])))
+ :query-fn (fn [op field-id] {:expressions {"expr" [op [:field field-id nil]]}
+ :aggregation [[:count]]
+ :breakout [[:expression "expr"]]})}])
+
+(deftest extraction-function-tests
+ (mt/dataset times-mixed
+ ;; need to have seperate tests for mongo because it doesn't have supports for casting yet
+ (mt/test-drivers (disj (mt/normal-drivers-with-feature :temporal-extract) :mongo)
+ (testing "with datetime columns"
+ (doseq [[col-type field-id] [[:datetime (mt/id :times :dt)] [:text-as-datetime (mt/id :times :as_dt)]]
+ op [:get-year :get-quarter :get-month :get-day
+ :get-day-of-week :get-hour :get-minute :get-second]
+ {:keys [expected-fn query-fn]}
+ extraction-test-cases]
+ (testing (format "extract %s function works as expected on %s column for driver %s" op col-type driver/*driver*)
+ (is (= (set (expected-fn op)) (set (test-temporal-extract (query-fn op field-id))))))))
+
+ (testing "with date columns"
+ (doseq [[col-type field-id] [[:date (mt/id :times :d)] [:text-as-date (mt/id :times :as_d)]]
+ op [:get-year :get-quarter :get-month :get-day :get-day-of-week]
+ {:keys [expected-fn query-fn]}
+ extraction-test-cases]
+ (testing (format "extract %s function works as expected on %s column for driver %s" op col-type driver/*driver*)
+ (is (= (set (expected-fn op)) (set (test-temporal-extract (query-fn op field-id)))))))))
+
+ ;; need to have seperate tests for mongo because it doesn't have supports for casting yet
+ (mt/test-driver :mongo
+ (testing "with datetimes columns"
+ (let [[col-type field-id] [:datetime (mt/id :times :dt)]]
+ (doseq [op [:get-year :get-quarter :get-month :get-day
+ :get-day-of-week :get-hour :get-minute :get-second]
+ {:keys [expected-fn query-fn]}
+ extraction-test-cases]
+ (testing (format "extract %s function works as expected on %s column for driver %s" op col-type driver/*driver*)
+ (is (= (set (expected-fn op)) (set (test-temporal-extract (query-fn op field-id)))))))))
+
+ (testing "with date columns"
+ (let [[col-type field-id] [:date (mt/id :times :d)]]
+ (doseq [op [:get-year :get-quarter :get-month :get-day :get-day-of-week]
+ {:keys [expected-fn query-fn]}
+ extraction-test-cases]
+ (testing (format "extract %s function works as expected on %s column for driver %s" op col-type driver/*driver*)
+ (is (= (set (expected-fn op)) (set (test-temporal-extract (query-fn op field-id))))))))))))
+
+
+(deftest temporal-extraction-with-filter-expresion-tests
+ (mt/test-drivers (mt/normal-drivers-with-feature :temporal-extract)
+ (mt/dataset times-mixed
+ (doseq [{:keys [title expected query]}
+ [{:title "Nested expression"
+ :expected [2004]
+ :query {:expressions {"expr" [:abs [:get-year [:field (mt/id :times :dt) nil]]]}
+ :filter [:= [:field (mt/id :times :index) nil] 1]
+ :fields [[:expression "expr"]]}}
+
+ {:title "Nested with arithmetic"
+ :expected [4008]
+ :query {:expressions {"expr" [:* [:get-year [:field (mt/id :times :dt) nil]] 2]}
+ :filter [:= [:field (mt/id :times :index) nil] 1]
+ :fields [[:expression "expr"]]}}
+
+ {:title "Filter using the extracted result - equality"
+ :expected [1]
+ :query {:filter [:= [:get-year [:field (mt/id :times :dt) nil]] 2004]
+ :fields [[:field (mt/id :times :index) nil]]}}
+
+ {:title "Filter using the extracted result - comparable"
+ :expected [1]
+ :query {:filter [:< [:get-year [:field (mt/id :times :dt) nil]] 2005]
+ :fields [[:field (mt/id :times :index) nil]]}}
+
+ {:title "Nested expression in fitler"
+ :expected [1]
+ :query {:filter [:= [:* [:get-year [:field (mt/id :times :dt) nil]] 2] 4008]
+ :fields [[:field (mt/id :times :index) nil]]}}]]
+ (testing title
+ (is (= expected (test-temporal-extract query))))))))
--
GitLab