Skip to content

fix: keep DuckDB array-slice colon dense inside [ ] #960

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.

Sign up for GitHub

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

Open
gaoflow wants to merge 1 commit into
base: master
Choose a base branch
from
+35 −9
Open

fix: keep DuckDB array-slice colon dense inside [ ] #960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions src/formatter/ExpressionFormatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ interface ExpressionFormatterParams {
params: Params;
layout: Layout;
inline?: boolean;
// openParen of the nearest enclosing bracket ("(", "[" or "{"), if any
enclosingBracket?: string;
}

export interface DialectFormatOptions {
Expand Down Expand Up @@ -76,13 +78,22 @@ export default class ExpressionFormatter {
private inline = false;
private nodes: AstNode[] = [];
private index = -1;

constructor({ cfg, dialectCfg, params, layout, inline = false }: ExpressionFormatterParams) {
private enclosingBracket?: string;

constructor({
cfg,
dialectCfg,
params,
layout,
inline = false,
enclosingBracket,
}: ExpressionFormatterParams) {
this.cfg = cfg;
this.dialectCfg = dialectCfg;
this.inline = inline;
this.params = params;
this.layout = layout;
this.enclosingBracket = enclosingBracket;
}

public format(nodes: AstNode[]): Layout {
Expand Down Expand Up @@ -194,7 +205,7 @@ export default class ExpressionFormatter {
}

private formatParenthesis(node: ParenthesisNode) {
const inlineLayout = this.formatInlineExpression(node.children);
const inlineLayout = this.formatInlineExpression(node.children, node.openParen);

if (inlineLayout) {
this.layout.add(node.openParen);
Expand All @@ -205,11 +216,11 @@ export default class ExpressionFormatter {

if (isTabularStyle(this.cfg)) {
this.layout.add(WS.INDENT);
this.layout = this.formatSubExpression(node.children);
this.layout = this.formatSubExpression(node.children, node.openParen);
} else {
this.layout.indentation.increaseBlockLevel();
this.layout.add(WS.INDENT);
this.layout = this.formatSubExpression(node.children);
this.layout = this.formatSubExpression(node.children, node.openParen);
this.layout.indentation.decreaseBlockLevel();
}

Expand Down Expand Up @@ -346,6 +357,9 @@ export default class ExpressionFormatter {
this.layout.add(text, WS.SPACE);
} else if (this.cfg.denseOperators || this.dialectCfg.alwaysDenseOperators.includes(text)) {
this.layout.add(WS.NO_SPACE, text);
} else if (text === ':' && this.enclosingBracket === '[') {
// Array slice colon (e.g. arr[1:5]) is dense; the key-value ":" keeps its space.
this.layout.add(WS.NO_SPACE, text);
} else if (text === ':') {
this.layout.add(WS.NO_SPACE, text, WS.SPACE);
} else {
Expand Down Expand Up @@ -471,17 +485,21 @@ export default class ExpressionFormatter {
}
}

private formatSubExpression(nodes: AstNode[]): Layout {
private formatSubExpression(nodes: AstNode[], enclosingBracket = this.enclosingBracket): Layout {
return new ExpressionFormatter({
cfg: this.cfg,
dialectCfg: this.dialectCfg,
params: this.params,
layout: this.layout,
inline: this.inline,
enclosingBracket,
}).format(nodes);
}

private formatInlineExpression(nodes: AstNode[]): Layout | undefined {
private formatInlineExpression(
nodes: AstNode[],
enclosingBracket = this.enclosingBracket
): Layout | undefined {
const oldParamIndex = this.params.getPositionalParameterIndex();
try {
return new ExpressionFormatter({
Expand All @@ -490,6 +508,7 @@ export default class ExpressionFormatter {
params: this.params,
layout: new InlineLayout(this.cfg.expressionWidth),
inline: true,
enclosingBracket,
}).format(nodes);
} catch (e) {
if (e instanceof InlineLayoutError) {
Expand Down
11 changes: 9 additions & 2 deletions test/duckdb.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,7 @@ describe('DuckDBFormatter', () => {
`);
});

// TODO: This currently conflicts with ":"-operator in struct literals
it.skip('supports array slice operator', () => {
it('supports array slice operator', () => {
expect(format('SELECT foo[:5], bar[1:], baz[1:5], zap[:];')).toBe(dedent`
SELECT
foo[:5],
Expand All @@ -171,6 +170,14 @@ describe('DuckDBFormatter', () => {
`);
});

it('denses array-slice colon while keeping struct/list colon spaced', () => {
expect(format("SELECT {'k': list[1:5]}, arr[i:j];")).toBe(dedent`
SELECT
{'k': list[1:5]},
arr[i:j];
`);
});

// TODO: This currently conflicts with the modulo operator
it.skip('formats percentage value in LIMIT clause', () => {
expect(format('SELECT * FROM foo LIMIT 10%;')).toBe(dedent`
Expand Down