Skip to content

Commit

Permalink
datacube: improve roundtrip (#3820)
Browse files Browse the repository at this point in the history
* datacube: allow choosing vX_X_X protocol version for execution

* datacube: support type-checking for leaf-level extend()

* datacube: roundtrip test for extend(), sort(), limit(), select()

* datacube: verify variable when processing lambdas

* datacube: roundtrip processing and test for aggregation

* datacube: support sorting pivot result columns

* datacube: roundtrip tests for pivot and duplicate columns check

* datacube: more validations for groupBy and pivot

* datacube: roundtrip check for configuration
  • Loading branch information
akphi authored Jan 23, 2025
1 parent 948cbf4 commit 4bccf07
Show file tree
Hide file tree
Showing 89 changed files with 3,966 additions and 1,517 deletions.
7 changes: 7 additions & 0 deletions .changeset/large-lemons-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@finos/legend-application-data-cube': patch
'@finos/legend-application-repl': patch
'@finos/legend-query-builder': patch
'@finos/legend-data-cube': patch
'@finos/legend-graph': patch
---
7 changes: 7 additions & 0 deletions .changeset/perfect-planets-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@finos/legend-application-data-cube': patch
'@finos/legend-application-repl': patch
'@finos/legend-query-builder': patch
'@finos/legend-data-cube': patch
'@finos/legend-graph': patch
---
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
_serializeValueSpecification,
_deserializeValueSpecification,
_defaultPrimitiveTypeValue,
type DataCubeExecutionOptions,
} from '@finos/legend-data-cube';
import {
isNonNullable,
Expand Down Expand Up @@ -224,6 +225,46 @@ export class LegendDataCubeDataCubeEngine extends DataCubeEngine {
}
}

override async parseValueSpecification(
code: string,
returnSourceInformation?: boolean | undefined,
) {
try {
return _deserializeValueSpecification(
await this._engineServerClient.grammarToJSON_valueSpecification(
code,
'',
undefined,
undefined,
returnSourceInformation,
),
);
} catch (error) {
assertErrorThrown(error);
if (
error instanceof NetworkClientError &&
error.response.status === HttpStatus.BAD_REQUEST
) {
throw V1_buildParserError(
V1_ParserError.serialization.fromJson(
error.payload as PlainObject<V1_ParserError>,
),
);
}
throw error;
}
}

override async getValueSpecificationCode(
value: V1_ValueSpecification,
pretty?: boolean | undefined,
) {
return this._graphManager.valueSpecificationToPureCode(
_serializeValueSpecification(value),
pretty,
);
}

// TODO: we could optimize this by synthesizing the base query from the source columns
// instead of having to send the entire graph model
override async getQueryTypeahead(
Expand Down Expand Up @@ -256,46 +297,32 @@ export class LegendDataCubeDataCubeEngine extends DataCubeEngine {
);
}

override async parseValueSpecification(
code: string,
returnSourceInformation?: boolean | undefined,
override async getQueryRelationReturnType(
query: V1_Lambda,
source: DataCubeSource,
) {
try {
return _deserializeValueSpecification(
await this._engineServerClient.grammarToJSON_valueSpecification(
code,
'',
undefined,
undefined,
returnSourceInformation,
),
return await this._getQueryRelationType(
_serializeValueSpecification(query),
source,
);
} catch (error) {
assertErrorThrown(error);
if (
error instanceof NetworkClientError &&
error.response.status === HttpStatus.BAD_REQUEST
) {
throw V1_buildParserError(
V1_ParserError.serialization.fromJson(
error.payload as PlainObject<V1_ParserError>,
const engineError = V1_buildEngineError(
V1_EngineError.serialization.fromJson(
error.payload as PlainObject<V1_EngineError>,
),
);
throw engineError;
}
throw error;
}
}

override async getValueSpecificationCode(
value: V1_ValueSpecification,
pretty?: boolean | undefined,
) {
return this._graphManager.valueSpecificationToPureCode(
_serializeValueSpecification(value),
pretty,
);
}

// TODO: we could optimize this by synthesizing the base query from the source columns
// instead of having to send the entire graph model
override async getQueryCodeRelationReturnType(
Expand Down Expand Up @@ -335,17 +362,22 @@ export class LegendDataCubeDataCubeEngine extends DataCubeEngine {
}
}

override async executeQuery(query: V1_Lambda, source: DataCubeSource) {
override async executeQuery(
query: V1_Lambda,
source: DataCubeSource,
options?: DataCubeExecutionOptions | undefined,
) {
const queryCodePromise = this.getValueSpecificationCode(query);
let result: ExecutionResult;
if (source instanceof AdhocQueryDataCubeSource) {
result = await this._runQuery(query, source.model);
result = await this._runQuery(query, source.model, undefined, options);
} else if (source instanceof LegendQueryDataCubeSource) {
query.parameters = source.lambda.parameters;
result = await this._runQuery(
query,
source.model,
source.parameterValues,
options,
);
} else {
throw new UnsupportedOperationError(
Expand Down Expand Up @@ -429,15 +461,17 @@ export class LegendDataCubeDataCubeEngine extends DataCubeEngine {
query: V1_Lambda,
model: PlainObject<V1_PureModelContext>,
parameterValues?: V1_ParameterValue[] | undefined,
options?: DataCubeExecutionOptions | undefined,
): Promise<ExecutionResult> {
return V1_buildExecutionResult(
V1_deserializeExecutionResult(
(await this._engineServerClient.runQuery({
clientVersion:
options?.clientVersion ??
// eslint-disable-next-line no-process-env
process.env.NODE_ENV === 'development'
(process.env.NODE_ENV === 'development'
? PureClientVersion.VX_X_X
: undefined,
: undefined),
function: _serializeValueSpecification(query),
model,
context: serialize(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,13 @@ export class LegendREPLDataCubeEngine extends DataCubeEngine {
});
}

override async getQueryRelationReturnType(
query: V1_Lambda,
source: DataCubeSource,
) {
return this._getQueryRelationType(query);
}

override async getQueryCodeRelationReturnType(
code: string,
baseQuery: V1_ValueSpecification,
Expand Down
2 changes: 2 additions & 0 deletions packages/legend-data-cube/docs/column-configuration.kind.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ id: 'data-cube.column-configuration.kind'

A column represents either a `dimension` or a `measure`.

> Column kind cannot be changed while the column is used in pivot.
# Dimension

Descriptions that help group and filter data. Typically associated with string columns where each value represents a category.
Expand Down
1 change: 1 addition & 0 deletions packages/legend-data-cube/src/__lib__/DataCubeSetting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

export enum DataCubeSettingKey {
DEBUGGER__ENABLE_DEBUG_MODE = 'dataCube.debugger.enableDebugMode',
DEBUGGER__USE_DEV_CLIENT_PROTOCOL_VERSION = 'dataCube.debugger.useDevClientProtocolVersion',
DEBUGGER__ACTION__RELOAD = 'dataCube.debugger.action.reload',
GRID_CLIENT__ROW_BUFFER = 'dataCube.grid.rowBuffer',
GRID_CLIENT__PURGE_CLOSED_ROW_NODES = 'dataCube.grid.purgeClosedRowNodes',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ import {
} from '../../../stores/core/DataCubeQueryEngine.js';
import { DataCubeDocumentationKey } from '../../../__lib__/DataCubeDocumentation.js';
import type { DataCubeViewState } from '../../../stores/view/DataCubeViewState.js';
import { _sortByColName } from '../../../stores/core/model/DataCubeColumn.js';
import {
_findCol,
_sortByColName,
} from '../../../stores/core/model/DataCubeColumn.js';

export const DataCubeEditorColumnPropertiesPanel = observer(
(props: { view: DataCubeViewState }) => {
Expand Down Expand Up @@ -150,17 +153,16 @@ export const DataCubeEditorColumnPropertiesPanel = observer(
{selectedColumn.dataType}
</div>
{Boolean(
editor.leafExtendColumns.find(
(col) => col.name === selectedColumn.name,
),
_findCol(editor.leafExtendColumns, selectedColumn.name),
) && (
<div className="mr-0.5 flex h-3.5 flex-shrink-0 items-center rounded-sm border border-neutral-300 bg-neutral-100 px-1 text-xs font-medium uppercase text-neutral-600">
{`Extended (Leaf Level)`}
</div>
)}
{Boolean(
editor.groupExtendColumns.find(
(col) => col.name === selectedColumn.name,
_findCol(
editor.groupExtendColumns,
selectedColumn.name,
),
) && (
<div className="mr-0.5 flex h-3.5 flex-shrink-0 items-center rounded-sm border border-neutral-300 bg-neutral-100 px-1 text-xs font-medium uppercase text-neutral-600">
Expand Down Expand Up @@ -188,18 +190,14 @@ export const DataCubeEditorColumnPropertiesPanel = observer(
{column.dataType}
</div>
{Boolean(
editor.leafExtendColumns.find(
(col) => col.name === column.name,
),
_findCol(editor.leafExtendColumns, column.name),
) && (
<div className="mr-0.5 flex h-3.5 flex-shrink-0 items-center rounded-sm border border-neutral-300 bg-neutral-100 px-1 text-xs font-medium uppercase text-neutral-600">
{`Extended (Leaf Level)`}
</div>
)}
{Boolean(
editor.groupExtendColumns.find(
(col) => col.name === column.name,
),
_findCol(editor.groupExtendColumns, column.name),
) && (
<div className="mr-0.5 flex h-3.5 flex-shrink-0 items-center rounded-sm border border-neutral-300 bg-neutral-100 px-1 text-xs font-medium uppercase text-neutral-600">
{`Extended (Group Level)`}
Expand Down Expand Up @@ -227,13 +225,29 @@ export const DataCubeEditorColumnPropertiesPanel = observer(
open={kindDropdownPropsOpen}
// disallow changing the column kind if the column is being used as pivot column
disabled={Boolean(
editor.verticalPivots.selector.selectedColumns.find(
(col) => col.name === selectedColumn.name,
_findCol(
editor.verticalPivots.selector.selectedColumns,
selectedColumn.name,
) ??
editor.horizontalPivots.selector.selectedColumns.find(
(col) => col.name === selectedColumn.name,
_findCol(
editor.horizontalPivots.selector.selectedColumns,
selectedColumn.name,
),
)}
title={
Boolean(
_findCol(
editor.verticalPivots.selector.selectedColumns,
selectedColumn.name,
) ??
_findCol(
editor.horizontalPivots.selector.selectedColumns,
selectedColumn.name,
),
)
? 'Column kind cannot be changed while the column is used in pivot'
: undefined
}
>
{selectedColumn.kind}
</FormDropdownMenuTrigger>
Expand Down Expand Up @@ -314,7 +328,14 @@ export const DataCubeEditorColumnPropertiesPanel = observer(
<FormDropdownMenuItem
key={op.operator}
onClick={() => {
selectedColumn.setAggregateOperation(op);
if (op !== selectedColumn.aggregateOperation) {
selectedColumn.setAggregateOperation(op);
selectedColumn.setAggregationParameters(
op.generateDefaultParameterValues(
selectedColumn,
),
);
}
closeAggregationOperationDropdown();
}}
autoFocus={op === selectedColumn.aggregateOperation}
Expand All @@ -324,6 +345,8 @@ export const DataCubeEditorColumnPropertiesPanel = observer(
))}
</FormDropdownMenu>

{/* TODO: Support editing aggregation parameter values */}

<FormCheckbox
className="ml-3"
label="Exclude from horizontal pivot"
Expand Down
Loading

0 comments on commit 4bccf07

Please sign in to comment.