-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Snapshot building for filter operations (part of roundtrip) #3737
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: e17a1b3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
...s/legend-data-cube/src/stores/core/__tests__/DataCubeQueryFilterSnapshotBuilder-repl.test.ts
Outdated
Show resolved
Hide resolved
...s/legend-data-cube/src/stores/core/__tests__/DataCubeQueryFilterSnapshotBuilder-repl.test.ts
Outdated
Show resolved
Hide resolved
packages/legend-data-cube/src/stores/core/filter/DataCubeQueryFilterOperation.ts
Outdated
Show resolved
Hide resolved
packages/legend-data-cube/src/stores/core/filter/DataCubeQueryFilterOperation.ts
Outdated
Show resolved
Hide resolved
packages/legend-data-cube/src/stores/core/DataCubeQueryEngine.ts
Outdated
Show resolved
Hide resolved
packages/legend-data-cube/src/stores/core/DataCubeQuerySnapshotBuilderUtils.ts
Outdated
Show resolved
Hide resolved
packages/legend-data-cube/src/stores/core/DataCubeQuerySnapshotBuilderUtils.ts
Outdated
Show resolved
Hide resolved
packages/legend-data-cube/src/stores/core/DataCubeQuerySnapshotBuilder.ts
Show resolved
Hide resolved
packages/legend-data-cube/src/stores/core/DataCubeQuerySnapshotBuilderUtils.ts
Outdated
Show resolved
Hide resolved
packages/legend-data-cube/src/stores/core/DataCubeQuerySnapshotBuilderUtils.ts
Outdated
Show resolved
Hide resolved
...s/legend-data-cube/src/stores/core/__tests__/DataCubeQueryFilterSnapshotBuilder-repl.test.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3737 +/- ##
==========================================
+ Coverage 44.88% 46.15% +1.27%
==========================================
Files 2229 2229
Lines 385911 386183 +272
Branches 15874 16473 +599
==========================================
+ Hits 173204 178262 +5058
+ Misses 211943 207480 -4463
+ Partials 764 441 -323
|
packages/legend-data-cube/src/stores/core/__tests__/DataCubeQuerySnapshotBuilder.repl-test.ts
Outdated
Show resolved
Hide resolved
packages/legend-data-cube/src/stores/core/__tests__/DataCubeQuerySnapshotBuilder.repl-test.ts
Outdated
Show resolved
Hide resolved
...s/legend-data-cube/src/stores/core/__tests__/DataCubeQueryFilterSnapshotBuilder.repl-test.ts
Outdated
Show resolved
Hide resolved
...s/legend-data-cube/src/stores/core/__tests__/DataCubeQueryFilterSnapshotBuilder.repl-test.ts
Outdated
Show resolved
Hide resolved
efc0e74
to
5887c4e
Compare
@@ -234,6 +234,21 @@ export enum DataCubeQueryFilterOperator { | |||
// TODO?: having, having in aggregate, between | |||
} | |||
|
|||
export enum DataCubeEngineFilterOperator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use DataCubeFunction
return undefined; | ||
} | ||
|
||
function _buildFilterConditionSnapshot( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as discussed, distribute this logic to each of the operator. For common logic that can be shared, we can leave them here, e.g. _not(), etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this is the code for conditionExpression:
const filterCondition = filter;
const operation = filterOperations.find(
(op) => op.operator === filterCondition.operator,
);
const condition = operation?.buildConditionExpression(filterCondition);
operator for most cases is the same but different for some cases so will need switch case still to put in the operator but I get the comment
throw new Error('Method not implemented.'); | ||
} | ||
|
||
static filterOperations = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's remove this. This inherits the list from DataCubeEngine
.
[ | ||
_filter( | ||
snapshot.data.filter!, | ||
Test__DataCubeEngine.filterOperations, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test__DataCubeEngine.filterOperations, | |
const engine = new Test__DataCubeEngine(); | |
... | |
engine.filterOperations, |
Summary
How did you test this change?