Skip to content

Commit d5da121

Browse files
authored
fix(tesseract): align bridge member SQL with JS proxy reference (cube-js#10850)
1 parent 2529852 commit d5da121

7 files changed

Lines changed: 355 additions & 275 deletions

File tree

packages/cubejs-backend-native/test/bridge/args-names.test.ts

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,51 +29,34 @@ describeBridge('bridge: args_names parser', () => {
2929
});
3030
});
3131

32-
// The skipped tests below assert how a correct parser should behave.
33-
// They are buggy today because both this bridge and the JS-side
34-
// schema-compiler use the same regex; a fix must touch both.
35-
describe('known bugs (skip = expected behavior after fix)', () => {
36-
// The named-function branch of the regex captures [A-Za-z0-9_,]* —
37-
// no whitespace allowed. V8 renders `function f(x, y)` with a space
38-
// after the comma, so capture stops after the first arg.
39-
it.skip('parses named function declaration with multiple args', () => {
32+
describe('edge cases', () => {
33+
it('parses named function declaration with multiple args', () => {
4034
function named(x: any, y: any) {
4135
return [x, y];
4236
}
4337
expect(parseArgsNames(named)).toEqual(['x', 'y']);
4438
});
4539

46-
it.skip('parses async named function declaration with multiple args', () => {
40+
it('parses async named function declaration with multiple args', () => {
4741
async function named(x: any, y: any) {
4842
return [x, y];
4943
}
5044
expect(parseArgsNames(named)).toEqual(['x', 'y']);
5145
});
5246

53-
// Default args land inside the (...) capture as a single token "x = 1"
54-
// and survive the comma split unchanged. Special names like
55-
// SECURITY_CONTEXT in this position fail to dispatch.
56-
it.skip('parses default args, returning just the identifier', () => {
47+
it('parses default args, returning just the identifier', () => {
5748
expect(parseArgsNames((x: any = 1) => x)).toEqual(['x']);
5849
});
5950

60-
// Rest args keep their leading dots after capture+split, yielding
61-
// "...args" instead of "args".
62-
it.skip('parses rest args, dropping the spread dots', () => {
51+
it('parses rest args, dropping the spread dots', () => {
6352
expect(parseArgsNames((...args: any[]) => args)).toEqual(['args']);
6453
});
6554

66-
// Destructuring patterns get split by the comma inside the braces,
67-
// breaking the pattern into half-tokens.
68-
it.skip('parses destructuring args, returning the destructured identifiers', () => {
55+
it('parses destructuring args, returning the destructured identifiers', () => {
6956
expect(parseArgsNames(({ a, b }: any) => [a, b])).toEqual(['a', 'b']);
7057
});
7158

72-
// Anonymous function expressions (`function (x){}`) match no branch
73-
// of the regex at all. Today Rust silently returns []; the JS side
74-
// throws `Can't match args for: ...`. Neither is the desired
75-
// behavior — the parser should just return the args.
76-
it.skip('parses anonymous function expressions', () => {
59+
it('parses anonymous function expressions', () => {
7760
// eslint-disable-next-line func-names, prefer-arrow-callback
7861
const fn = function (x: any) { return x; };
7962
expect(parseArgsNames(fn)).toEqual(['x']);

packages/cubejs-backend-native/test/bridge/multi-arg.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ describeBridge('bridge: multi-arg dispatch', () => {
1212
);
1313

1414
expect(result.template).toBe(
15-
'SUM({arg:0}) WHERE {fp:0} AND t = {sv:1}'
15+
'SUM({arg:0}) WHERE {fp:0} AND t = {sv:0}'
1616
);
1717
expect(result.args.symbol_paths).toEqual([['CUBE', 'amount']]);
1818
expect(result.args.filter_params).toEqual([
1919
{ cube_name: 'orders', name: 'status', column: 'col' },
2020
]);
21-
expect(result.args.security_context.values).toEqual(['acme', 'acme']);
21+
expect(result.args.security_context.values).toEqual(['acme']);
2222
});
2323

2424
it('throws an internal error from the StubBaseTools when SQL_UTILS is referenced', () => {

packages/cubejs-backend-native/test/bridge/result-shape.test.ts

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,20 @@ describeBridge('bridge: result shape', () => {
2626
expect(result.args.symbol_paths).toEqual([['orders', 'status']]);
2727
});
2828

29-
it('errors when the user function returns a primitive number', () => {
30-
// Bug: bridge eagerly coerces the return value via convert_to_string,
31-
// which only handles JsString and null — it then tries to call
32-
// toString as a struct method, which errors on primitives. JS
33-
// reference returns the value unchanged and lets downstream template
34-
// literals coerce naturally. See skipped 'JS-ref: numeric return…'.
35-
expect(() => compileMemberSql(() => 42 as any)).toThrow(
36-
/Object is not the Struct object/
37-
);
29+
it('coerces a numeric return to its string form', () => {
30+
const integerResult = compileMemberSql(() => 42 as any);
31+
const decimalResult = compileMemberSql(() => 1.5 as any);
32+
33+
expect(integerResult.template).toBe('42');
34+
expect(decimalResult.template).toBe('1.5');
3835
});
3936

40-
it('errors when the user function returns a primitive boolean', () => {
41-
expect(() => compileMemberSql(() => true as any)).toThrow(
42-
/Object is not the Struct object/
43-
);
37+
it('coerces a boolean return to its string form', () => {
38+
const trueResult = compileMemberSql(() => true as any);
39+
const falseResult = compileMemberSql(() => false as any);
40+
41+
expect(trueResult.template).toBe('true');
42+
expect(falseResult.template).toBe('false');
4443
});
4544

4645
it('returns an empty string template when the user function returns null', () => {
@@ -49,17 +48,4 @@ describeBridge('bridge: result shape', () => {
4948
expect(result.template).toBe('');
5049
expect(result.args.symbol_paths).toEqual([]);
5150
});
52-
53-
// JS reference does no coercion at the bridge boundary — the user
54-
// function's return value flows through resolveSymbolsCall verbatim and
55-
// is later embedded via template literal, which yields String(value).
56-
it.skip('JS-ref: numeric return is coerced to its string form', () => {
57-
const result = compileMemberSql(() => 42 as any);
58-
expect(result.template).toBe('42');
59-
});
60-
61-
it.skip('JS-ref: boolean return is coerced to its string form', () => {
62-
const result = compileMemberSql(() => true as any);
63-
expect(result.template).toBe('true');
64-
});
6551
});

0 commit comments

Comments
 (0)