Skip to content

Commit

Permalink
refactor(query): clean up cluster query (#1115)
Browse files Browse the repository at this point in the history
  • Loading branch information
EagleoutIce authored Oct 29, 2024
1 parent 38c4d43 commit 1c2cc65
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 15 deletions.
6 changes: 3 additions & 3 deletions src/cli/repl/commands/repl-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import { DEFAULT_DATAFLOW_PIPELINE } from '../../../core/steps/pipeline/default-
import { fileProtocol, requestFromInput } from '../../../r-bridge/retriever';
import type { ReplCommand, ReplOutput } from './repl-main';
import { splitAtEscapeSensitive } from '../../../util/args';
import { italic } from '../../../util/ansi';
import { ansiFormatter, italic } from '../../../util/ansi';
import { describeSchema } from '../../../util/schema';
import type { Query, QueryResults, SupportedQueryTypes } from '../../../queries/query';
import { AnyQuerySchema, QueriesSchema , executeQueries } from '../../../queries/query';
import type { PipelineOutput } from '../../../core/steps/pipeline/pipeline';
import { jsonReplacer } from '../../../util/json';
import { asciiSummaryOfQueryResult } from '../../../queries/query-print';


async function getDataflow(shell: RShell, remainingLine: string) {
Expand Down Expand Up @@ -72,8 +73,7 @@ export const queryCommand: ReplCommand = {
const results = await processQueryArgs(remainingLine, shell, output);
const totalEnd = Date.now();
if(results) {
output.stdout(JSON.stringify(results));
output.stdout('Total time: ' + (totalEnd - totalStart) + 'ms');
output.stdout(asciiSummaryOfQueryResult(ansiFormatter, totalEnd - totalStart, results.query, results.processed));
}
}
};
Expand Down
12 changes: 8 additions & 4 deletions src/queries/catalog/cluster-query/cluster-query-format.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { BaseQueryFormat, BaseQueryResult } from '../../base-query-format';
import { bold } from '../../../util/ansi';
import { bold, markdownFormatter } from '../../../util/ansi';
import Joi from 'joi';
import type { QueryResults, SupportedQuery } from '../../query';
import type { DataflowGraphClusters } from '../../../dataflow/cluster';
Expand Down Expand Up @@ -27,9 +27,13 @@ export const ClusterQueryDefinition = {
result.push(` ╰ Found ${out.clusters.length} cluster${out.clusters.length === 1 ? '' : 's'}`);
for(const cluster of out.clusters) {
const unknownSideEffects = cluster.hasUnknownSideEffects ? '(has unknown side effect)' : '';
result.push(` ╰ ${unknownSideEffects} {${summarizeIdsIfTooLong(cluster.members)}} ([marked](${
graphToMermaidUrl(processed.dataflow.graph, false, new Set(cluster.members))
}))`);
let suffix = '';
if(formatter === markdownFormatter) {
suffix = `([marked](${
graphToMermaidUrl(processed.dataflow.graph, false, new Set(cluster.members))
}))`;
}
result.push(` ╰ ${unknownSideEffects} {${summarizeIdsIfTooLong(formatter, cluster.members)}} ${suffix}`);
}
return true;
},
Expand Down
2 changes: 1 addition & 1 deletion src/queries/catalog/id-map-query/id-map-query-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const IdMapQueryDefinition = {
asciiSummarizer: (formatter, _processed, queryResults, result) => {
const out = queryResults as QueryResults<'id-map'>['id-map'];
result.push(`Query: ${bold('id-map', formatter)} (${printAsMs(out['.meta'].timing, 0)})`);
result.push(` ╰ Id List: {${summarizeIdsIfTooLong([...out.idMap.keys()])}}`);
result.push(` ╰ Id List: {${summarizeIdsIfTooLong(formatter, [...out.idMap.keys()])}}`);
return true;
},
schema: Joi.object({
Expand Down
2 changes: 1 addition & 1 deletion src/queries/catalog/lineage-query/lineage-query-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const LineageQueryDefinition = {
const out = queryResults as QueryResults<'lineage'>['lineage'];
result.push(`Query: ${bold('lineage', formatter)} (${printAsMs(out['.meta'].timing, 0)})`);
for(const [criteria, lineage] of Object.entries(out.lineages)) {
result.push(` ╰ ${criteria}: {${summarizeIdsIfTooLong([...lineage])}}`);
result.push(` ╰ ${criteria}: {${summarizeIdsIfTooLong(formatter, [...lineage])}}`);
}
return true;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const LocationMapQueryDefinition = {
asciiSummarizer: (formatter: OutputFormatter, _processed: unknown, queryResults: BaseQueryResult, result: string[]) => {
const out = queryResults as LocationMapQueryResult;
result.push(`Query: ${bold('location-map', formatter)} (${printAsMs(out['.meta'].timing, 0)})`);
result.push(` ╰ Id List: {${summarizeIdsIfTooLong([...Object.keys(out.map)])}}`);
result.push(` ╰ Id List: {${summarizeIdsIfTooLong(formatter, [...Object.keys(out.map)])}}`);
return true;
},
schema: Joi.object({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export const StaticSliceQueryDefinition = {
if('reconstruct' in obj) {
result.push(' ╰ Code (newline as <code>&#92;n</code>): <code>' + obj.reconstruct.code.split('\n').join('\\n') + '</code>');
} else {
result.push(` ╰ Id List: {${summarizeIdsIfTooLong([...obj.slice.result])}}`);
result.push(` ╰ Id List: {${summarizeIdsIfTooLong(formatter, [...obj.slice.result])}}`);
}
}
return true;
Expand Down
8 changes: 4 additions & 4 deletions src/queries/query-print.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { OutputFormatter } from '../util/ansi';
import { bold, italic } from '../util/ansi';
import { markdownFormatter , bold, italic } from '../util/ansi';
import type { QueryResults, SupportedQueryTypes } from './query';
import { SupportedQueries } from './query';
import type { PipelineOutput } from '../core/steps/pipeline/pipeline';
Expand Down Expand Up @@ -57,7 +57,7 @@ export function asciiCallContext(formatter: OutputFormatter, results: QueryResul
return result.join('\n');
}

export function summarizeIdsIfTooLong(ids: readonly NodeId[]) {
export function summarizeIdsIfTooLong(formatter: OutputFormatter, ids: readonly NodeId[]) {
const naive = ids.join(', ');
if(naive.length <= 20) {
return naive;
Expand All @@ -68,9 +68,9 @@ export function summarizeIdsIfTooLong(ids: readonly NodeId[]) {
acc += ids[i++] + ', ';
}
if(i < ids.length) {
acc += '... (see JSON below)';
acc += '... (see JSON)';
}
return textWithTooltip(acc, JSON.stringify(ids));
return formatter === markdownFormatter ? textWithTooltip(acc, JSON.stringify(ids)) : acc;
}

export function asciiSummaryOfQueryResult(formatter: OutputFormatter, totalInMs: number, results: QueryResults<SupportedQueryTypes>, processed: PipelineOutput<typeof DEFAULT_DATAFLOW_PIPELINE>): string {
Expand Down
3 changes: 3 additions & 0 deletions test/functionality/dataflow/graph/cluster-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ cat(product)
{ members: ['2@require', '2@vx'], hasUnknownSideEffects: true },
['1@x', '1@<-', '1@vx', '3@x']
]);
test('unknown side effects with forward ref', 'vx <- 3\nrequire(vx,character.only=TRUE)', [
{ members: ['1@vx', '1@<-', '1@3', '2@require', '2@vx', '$8', '2@TRUE'], hasUnknownSideEffects: true }
]);
});
}));
});
Expand Down

2 comments on commit 1c2cc65

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"artificial" Benchmark Suite

Benchmark suite Current: 1c2cc65 Previous: 96e933e Ratio
Retrieve AST from R code 246.33849572727271 ms (102.39516130654364) 246.50067822727272 ms (108.0778534921102) 1.00
Normalize R AST 17.790061772727274 ms (32.348833391369446) 17.822801272727272 ms (32.47501104567212) 1.00
Produce dataflow information 61.470147454545454 ms (131.55433810512932) 40.41399131818182 ms (86.99830114822697) 1.52
Total per-file 848.5164688181819 ms (1509.8642264475427) 829.6500495454545 ms (1498.0426737610057) 1.02
Static slicing 2.152730375523406 ms (1.3118657965425484) 2.0971564864344034 ms (1.2085738336418979) 1.03
Reconstruct code 0.24998477823715082 ms (0.19749964803799128) 0.24421041798721546 ms (0.1931884846808244) 1.02
Total per-slice 2.4169265883451154 ms (1.383358129798515) 2.356082671207842 ms (1.2841644653561477) 1.03
failed to reconstruct/re-parse 0 # 0 # 1
times hit threshold 0 # 0 # 1
reduction (characters) 0.7869360165281424 # 0.7869360165281424 # 1
reduction (normalized tokens) 0.7639690077689504 # 0.7639690077689504 # 1
memory (df-graph) 95.46617542613636 KiB (244.77619956879823) 95.46617542613636 KiB (244.77619956879823) 1

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"social-science" Benchmark Suite

Benchmark suite Current: 1c2cc65 Previous: 96e933e Ratio
Retrieve AST from R code 241.94994584 ms (45.61600408994183) 239.76002830000002 ms (44.476154875177215) 1.01
Normalize R AST 18.90854728 ms (14.32564320288246) 19.04045506 ms (14.770405721401682) 0.99
Produce dataflow information 73.19412526 ms (69.1300484597381) 74.39786466 ms (87.80796950166253) 0.98
Total per-file 7746.616322 ms (29005.994830041378) 7666.33215246 ms (28737.408915639426) 1.01
Static slicing 16.04049846747865 ms (44.26589044883673) 15.907723437298863 ms (43.83669809749617) 1.01
Reconstruct code 0.2672991721232062 ms (0.15740621046908668) 0.2509487217116593 ms (0.14943631432024615) 1.07
Total per-slice 16.31580967898284 ms (44.300443081540074) 16.166379499642126 ms (43.873427530614464) 1.01
failed to reconstruct/re-parse 0 # 0 # 1
times hit threshold 0 # 0 # 1
reduction (characters) 0.8712997340230448 # 0.8712997340230448 # 1
reduction (normalized tokens) 0.8102441553774778 # 0.8102441553774778 # 1
memory (df-graph) 99.47955078125 KiB (113.62321662710413) 99.8990234375 KiB (113.72812769327498) 1.00

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.