Skip to content

Commit

Permalink
Do not treat interpolated expressions as identifiers (#115)
Browse files Browse the repository at this point in the history
  • Loading branch information
crisptrutski authored Dec 11, 2024
1 parent cd8de13 commit 6ea7595
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 3 deletions.
4 changes: 4 additions & 0 deletions java/com/metabase/macaw/AstWalker.java
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,10 @@ public void visit(OverlapsCondition overlapsCondition) {

@Override
public <S> T visit(Column tableColumn, S context) {
// Ignore nested queries. In the future, we could do a nested part of them and merge the results.
if (tableColumn.getColumnName().startsWith("$$")) {
return null;
}
invokeCallback(COLUMN, tableColumn);

Table table = tableColumn.getTable();
Expand Down
2 changes: 1 addition & 1 deletion java/com/metabase/macaw/BasicTableExtractor.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private static Set<Table> getTables(PlainSelect select) {
tables.add(table);
} else if (item instanceof TableFunction) {
// Do not allow dynamic tables
throw new AnalysisError(AnalysisErrorType.INVALID_QUERY);
throw new AnalysisError(AnalysisErrorType.UNSUPPORTED_EXPRESSION);
} else {
// Only allow simple table references.
throw new AnalysisError(AnalysisErrorType.UNSUPPORTED_EXPRESSION);
Expand Down
2 changes: 1 addition & 1 deletion java/com/metabase/macaw/CompoundTableExtractor.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ private static void accTables(PlainSelect select, Set<Table> tables, Stack<Set<S
}
} else if (item instanceof TableFunction) {
// Do not allow dynamic tables
throw new AnalysisError(AnalysisErrorType.INVALID_QUERY);
throw new AnalysisError(AnalysisErrorType.UNSUPPORTED_EXPRESSION);
} else if (item instanceof Select) {
accTables((Select) item, tables, cteAliasScopes);
} else if (item != null) {
Expand Down
7 changes: 6 additions & 1 deletion test/macaw/acceptance_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,12 @@
(validate-analysis cv override actual-cv)))))
;; Testing path for newer modes.
(let [correct (:error expected-cs (:tables expected-cs))
override (if (str/includes? sql "-- BROKEN")
override (cond
(str/includes? sql "-- BROKEN")
:macaw.error/unable-to-parse
(str/includes? sql "-- UNSUPPORTED")
:macaw.error/unsupported-expression
:else
(get-override expected-cs m fixture :tables))
;; For now, we only support (and test) :tables
tables (testing (str prefix " table analysis does not throw for mode " m)
Expand Down Expand Up @@ -209,5 +213,6 @@
(test-fixture :compound/cte-nonambiguous)
(test-fixture :literal/with-table)
(test-fixture :literal/without-table)
(test-fixture :interpolation/crosstab)

(test-fixture :broken/filter-where))
10 changes: 10 additions & 0 deletions test/resources/acceptance/interpolation__crosstab.analysis.edn
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{:tables [{:table "history"}]
:source-columns [{:table "history" :column "id"}
{:table "history" :column "page"}
{:table "history" :column "h_timestamp"}]
:overrides
{:ast-walker-1
{:tables []
:source-columns []}
:basic-select :macaw.error/unsupported-expression
:compound-select :macaw.error/unsupported-expression}}
20 changes: 20 additions & 0 deletions test/resources/acceptance/interpolation__crosstab.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
SELECT * FROM crosstab($$

SELECT
history.page,
date_trunc('month', history.h_timestamp)::DATE,
count(history.id) as total
FROM history
WHERE h_timestamp between '2024-01-01' and '2024-12-01'
GROUP BY page, date_trunc('month', history.h_timestamp)
$$,

$$
SELECT
date_trunc('month', generate_series('2024-01-01', '2024-02-01', '1 month'::INTERVAL))::DATE
$$
) AS ct(
page INTEGER,
"Jan" FLOAT,
"Feb" FLOAT
)
2 changes: 2 additions & 0 deletions test/resources/acceptance/queries.dynamic.sql
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
-- FIXTURE: generate-series
-- UNSUPPORTED
SELECT t.day::date AS date
FROM generate_series(timestamp '2021-01-01', now(), interval '1 day') AS t(day)

-- FIXTURE: format
-- UNSUPPORTED
SELECT * FROM format('%I', table_name_variable);

-- FIXTURE: prepared-stmt
Expand Down

0 comments on commit 6ea7595

Please sign in to comment.