Skip to content

Commit 47710c1

Browse files
authored
add database name if exists on DB client (#2992)
<!-- CURSOR_SUMMARY --> > [!NOTE] > Adds database-qualified table handling across Python QueryClient and TypeScript sql tag, with new tests; updates backward-compat E2E to use latest CLI. > > - **Python (moose_lib)** > - **QueryClient.execute**: When interpolating `OlapTable` with a `database`, generates two identifier params (`{pN: Identifier}.{pN+1: Identifier}`) and updates preview formatting to `database.table`. > - Corrects parameter indexing and whitespace in function signature. > - **TypeScript (ts-moose-lib)** > - **sqlHelpers.Sql**: Renders `OlapTable` with `config.database` as `` `database`.`table` ``; falls back to `` `table` `` when no database. > - **Tests** > - Python: Add coverage for tables with/without `database` ensuring two-identifier parameterization. > - TypeScript: Add tests for database-qualified tables in `sql` template and an extra integration case. > - **E2E** > - Backward-compat test now uses `@514labs/moose-cli@latest` and updates log messages. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 7de28c3. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c33c52c commit 47710c1

File tree

5 files changed

+169
-8
lines changed

5 files changed

+169
-8
lines changed

apps/framework-cli-e2e/test/backward-compatibility.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ describe("Backward Compatibility Tests", function () {
305305
TEST_AWS_SECRET_ACCESS_KEY: "test-secret-access-key",
306306
};
307307

308-
devProcess = spawn("npx", ["-y", "@514labs/moose-cli@0.6.201", "dev"], {
308+
devProcess = spawn("npx", ["-y", "@514labs/moose-cli@latest", "dev"], {
309309
stdio: "pipe",
310310
cwd: TEST_PROJECT_DIR,
311311
env: devEnv,
@@ -317,7 +317,7 @@ describe("Backward Compatibility Tests", function () {
317317
SERVER_CONFIG.startupMessage,
318318
SERVER_CONFIG.url,
319319
);
320-
console.log("Server started with 0.6.201 CLI, infrastructure is ready");
320+
console.log("Server started with latest CLI, infrastructure is ready");
321321
// Brief wait to ensure everything is fully settled
322322
await setTimeoutAsync(5000);
323323

packages/py-moose-lib/moose_lib/main.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ def __init__(self, ch_client_or_config: Union[ClickhouseClient, RuntimeClickHous
185185
def __call__(self, input, variables):
186186
return self.execute(input, variables)
187187

188-
def execute(self, input: Union[str, Query], variables = None, row_type: Type[BaseModel] = None):
188+
def execute(self, input: Union[str, Query], variables=None, row_type: Type[BaseModel] = None):
189189
"""
190190
Execute a query.
191191
@@ -212,21 +212,30 @@ def execute(self, input: Union[str, Query], variables = None, row_type: Type[Bas
212212
values: dict[str, Any] = {}
213213
preview_params = {}
214214

215-
for i, (_, variable_name, _, _) in enumerate(Formatter().parse(input)):
215+
i = 0
216+
for _, variable_name, _, _ in Formatter().parse(input):
216217
if variable_name:
217218
value = variables[variable_name]
218219
if isinstance(value, list) and len(value) == 1:
219220
# handling passing the value of the query string dict directly to variables
220221
value = value[0]
221222

222223
if isinstance(value, Column) or isinstance(value, OlapTable):
223-
params[variable_name] = f'{{p{i}: Identifier}}'
224-
values[f'p{i}'] = value.name
224+
if isinstance(value, OlapTable) and value.config.database:
225+
params[variable_name] = f'{{p{i}: Identifier}}.{{p{i + 1}: Identifier}}'
226+
values[f'p{i}'] = value.config.database
227+
values[f'p{i + 1}'] = value.name
228+
i += 2
229+
else:
230+
params[variable_name] = f'{{p{i}: Identifier}}'
231+
values[f'p{i}'] = value.name
232+
i += 1
225233
else:
226234
from moose_lib.utilities.sql import clickhouse_param_type_for_value
227235
ch_type = clickhouse_param_type_for_value(value)
228236
params[variable_name] = f'{{p{i}: {ch_type}}}'
229237
values[f'p{i}'] = value
238+
i += 1
230239
preview_params[variable_name] = self._format_value_for_preview(value)
231240

232241
clickhouse_query = input.format_map(params)
@@ -290,6 +299,9 @@ def _format_value_for_preview(self, value: Any) -> str:
290299
if isinstance(value, datetime):
291300
return f"'{value.strftime('%Y-%m-%d %H:%M:%S')}'"
292301

302+
if isinstance(value, OlapTable) and value.config.database:
303+
return f"{value.config.database}.{value.name}"
304+
293305
if isinstance(value, Column) or isinstance(value, OlapTable):
294306
return value.name
295307

packages/py-moose-lib/tests/test_query_builder.py

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from datetime import datetime
22

33
from moose_lib.query_builder import Query, col
4-
from moose_lib.dmv2 import IngestPipeline, IngestPipelineConfig
4+
from moose_lib.dmv2 import IngestPipeline, IngestPipelineConfig, OlapTable, OlapConfig
55
from pydantic import BaseModel
66
from moose_lib.data_models import Key
77

@@ -36,3 +36,76 @@ def test_simple_select_and_where():
3636
assert params == {"p0": True}
3737

3838

39+
def test_table_with_database_config():
40+
"""Test that tables with database config generate correct SQL with two identifiers"""
41+
42+
class TestModel(BaseModel):
43+
id: int
44+
name: str
45+
46+
# Table without database
47+
table_without_db = OlapTable[TestModel]("my_table_no_db", OlapConfig())
48+
49+
# Table with database
50+
table_with_db = OlapTable[TestModel]("my_table_with_db", OlapConfig(database="my_database"))
51+
52+
# Test Query builder with table that has database
53+
q1 = Query().from_(table_with_db).select(table_with_db.cols.id, table_with_db.cols.name)
54+
sql1 = q1.to_sql()
55+
# The Query builder should handle the database-qualified table reference
56+
assert "my_database" in sql1 or "my_table" in sql1
57+
58+
# Test string interpolation format for QueryClient.execute()
59+
# When a table with database is used, it should generate two separate Identifier parameters
60+
from string import Formatter
61+
62+
# Simulate what happens in QueryClient.execute() with a table that has database
63+
template = "SELECT * FROM {table}"
64+
variables = {"table": table_with_db}
65+
66+
params = {}
67+
values = {}
68+
i = 0
69+
70+
for _, variable_name, _, _ in Formatter().parse(template):
71+
if variable_name:
72+
value = variables[variable_name]
73+
if isinstance(value, OlapTable) and value.config.database:
74+
# Should use two separate Identifier parameters
75+
params[variable_name] = f'{{p{i}: Identifier}}.{{p{i + 1}: Identifier}}'
76+
values[f'p{i}'] = value.config.database
77+
values[f'p{i + 1}'] = value.name
78+
i += 2
79+
else:
80+
params[variable_name] = f'{{p{i}: Identifier}}'
81+
values[f'p{i}'] = value.name
82+
i += 1
83+
84+
clickhouse_query = template.format_map(params)
85+
86+
assert clickhouse_query == "SELECT * FROM {p0: Identifier}.{p1: Identifier}"
87+
assert values == {"p0": "my_database", "p1": "my_table_with_db"}
88+
89+
# Test with table without database
90+
variables_no_db = {"table": table_without_db}
91+
params_no_db = {}
92+
values_no_db = {}
93+
i = 0
94+
95+
for _, variable_name, _, _ in Formatter().parse(template):
96+
if variable_name:
97+
value = variables_no_db[variable_name]
98+
if isinstance(value, OlapTable) and value.config.database:
99+
params_no_db[variable_name] = f'{{p{i}: Identifier}}.{{p{i + 1}: Identifier}}'
100+
values_no_db[f'p{i}'] = value.config.database
101+
values_no_db[f'p{i + 1}'] = value.name
102+
i += 2
103+
else:
104+
params_no_db[variable_name] = f'{{p{i}: Identifier}}'
105+
values_no_db[f'p{i}'] = value.name
106+
i += 1
107+
108+
clickhouse_query_no_db = template.format_map(params_no_db)
109+
110+
assert clickhouse_query_no_db == "SELECT * FROM {p0: Identifier}"
111+
assert values_no_db == {"p0": "my_table_no_db"}

packages/ts-moose-lib/src/sqlHelpers.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,11 @@ export class Sql {
127127
}
128128
this.strings[pos] += rawString;
129129
} else if (isTable(child)) {
130-
this.strings[pos] += `\`${child.name}\``;
130+
if (child.config.database) {
131+
this.strings[pos] += `\`${child.config.database}\`.\`${child.name}\``;
132+
} else {
133+
this.strings[pos] += `\`${child.name}\``;
134+
}
131135
this.strings[pos] += rawString;
132136
} else {
133137
this.values[pos++] = child;

packages/ts-moose-lib/tests/standalone.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { expect } from "chai";
22
import { getMooseClients } from "../src/consumption-apis/standalone";
33
import { sql } from "../src/sqlHelpers";
4+
import { OlapTable } from "../src/dmv2";
45

56
describe("BYOF Standalone Functionality", function () {
67
this.timeout(10000);
@@ -128,6 +129,77 @@ describe("BYOF Standalone Functionality", function () {
128129
});
129130
});
130131

132+
describe("sql template tag with database-qualified tables", () => {
133+
it("should handle table without database config", () => {
134+
interface TestModel1 {
135+
id: number;
136+
name: string;
137+
}
138+
139+
const table = new OlapTable<TestModel1>("table_no_db");
140+
const query = sql`SELECT * FROM ${table}`;
141+
142+
// Table without database should be rendered inline as `table_no_db`
143+
// The sql template tag concatenates the table directly into strings
144+
expect(query.strings.join("")).to.include("`table_no_db`");
145+
expect(query.values).to.have.lengthOf(0);
146+
});
147+
148+
it("should handle table with database config", () => {
149+
interface TestModel2 {
150+
id: number;
151+
name: string;
152+
}
153+
154+
const table = new OlapTable<TestModel2>("table_with_db", {
155+
database: "my_database",
156+
});
157+
const query = sql`SELECT * FROM ${table}`;
158+
159+
// Table with database should be rendered as `my_database`.`table_with_db`
160+
const fullQuery = query.strings.join("");
161+
expect(fullQuery).to.include("`my_database`.`table_with_db`");
162+
expect(query.values).to.have.lengthOf(0);
163+
});
164+
165+
it("should handle multiple tables with different database configs", () => {
166+
interface TestModel3 {
167+
id: number;
168+
name: string;
169+
}
170+
171+
const table1 = new OlapTable<TestModel3>("multi_table1", {
172+
database: "db1",
173+
});
174+
const table2 = new OlapTable<TestModel3>("multi_table2"); // no database
175+
const query = sql`SELECT * FROM ${table1} JOIN ${table2}`;
176+
177+
// Should properly interpolate both tables
178+
const fullQuery = query.strings.join("");
179+
expect(fullQuery).to.include("`db1`.`multi_table1`");
180+
expect(fullQuery).to.include("`multi_table2`");
181+
expect(query.values).to.have.lengthOf(0);
182+
});
183+
184+
it("should handle table in WHERE clause with database config", () => {
185+
interface TestModel4 {
186+
id: number;
187+
name: string;
188+
}
189+
190+
const table = new OlapTable<TestModel4>("events_table", {
191+
database: "analytics",
192+
});
193+
const userId = 123;
194+
const query = sql`SELECT * FROM ${table} WHERE user_id = ${userId}`;
195+
196+
const fullQuery = query.strings.join("");
197+
expect(fullQuery).to.include("`analytics`.`events_table`");
198+
expect(query.values).to.have.lengthOf(1);
199+
expect(query.values[0]).to.equal(123);
200+
});
201+
});
202+
131203
describe("Integration: getMooseClients + sql", () => {
132204
it("should work together to create queries", async () => {
133205
const { client } = await getMooseClients();

0 commit comments

Comments
 (0)