Skip to content

Commit

Permalink
Fix - diagnose-expression throws when Offset is nested (#42497)
Browse files Browse the repository at this point in the history
* Fix offset not working in case

* Make offset function return any

* Add a repro for #42377

* Fix order of adjustments

* Revert unrelated changes

* Remove commented code

* Revert unrelated changes
  • Loading branch information
kamilmielnik committed May 10, 2024
1 parent a522cd8 commit 3e9f4ac
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 3 deletions.
43 changes: 43 additions & 0 deletions e2e/test/scenarios/question/offset.cy.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
import type { StructuredQuestionDetails } from "e2e/support/helpers";
import {
createQuestion,
enterCustomColumnDetails,
getNotebookStep,
openNotebook,
popover,
restore,
} from "e2e/support/helpers";

const { ORDERS_ID } = SAMPLE_DATABASE;

describe("scenarios > question > offset", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
});

it("should allow using OFFSET as a CASE argument (metabase#42377)", () => {
const formula = "Sum(case([Total] > 0, Offset([Total], -1)))";
const name = "Aggregation";
const questionDetails: StructuredQuestionDetails = {
query: {
"source-table": ORDERS_ID,
limit: 5,
},
};
createQuestion(questionDetails, { visitQuestion: true });
openNotebook();

cy.icon("sum").click();
getNotebookStep("summarize")
.findByText("Pick the metric you want to see")
.click();
popover().contains("Custom Expression").click();
enterCustomColumnDetails({ formula, name });

cy.on("uncaught:exception", error => {
expect(error.message.includes("Error normalizing")).not.to.be.true;
});
});
});
2 changes: 1 addition & 1 deletion frontend/src/metabase-lib/v1/expressions/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ function prattCompiler({
passes: [
adjustOptions,
useShorthands,
adjustCase,
adjustOffset,
adjustCase,
expression =>
resolve({
expression,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,6 @@ export const parse = pipe(
recursiveParse,
adjustOptions,
useShorthands,
adjustCase,
adjustOffset,
adjustCase,
);
2 changes: 1 addition & 1 deletion frontend/test/metabase/lib/expressions/pratt/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface Opts {

export function compile(source: string, type: Type, opts: Opts = {}) {
const { throwOnError } = opts;
const passes = [adjustOptions, useShorthands, adjustCase, adjustOffset];
const passes = [adjustOptions, useShorthands, adjustOffset, adjustCase];
return newCompile(
parse(lexify(source), {
throwOnError,
Expand Down

0 comments on commit 3e9f4ac

Please sign in to comment.